From: Stephen Toub Date: Wed, 11 Nov 2020 14:15:06 +0000 (-0500) Subject: Remove some allocation at "hello world" startup (#44469) X-Git-Tag: submit/tizen/20210909.063632~4677 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=aa04371cabe4c82a39d993582cf7e14a07d1f1ab;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Remove some allocation at "hello world" startup (#44469) * Specialize `EqualityComparer.Default` Removes ~80 allocations at startup. * Avoid loading Encoding.Unicode just for CodePage * Use fixed instead of GCHandle.Alloc in EventSource.Initialize * Remove lock / lock object from EncodingProvider.AddProvider * Remove lock object from AppDomain.cs * Lazily allocate EventSource's m_createEventLock It's only used on an error path. We don't need to allocate it for each EventSource that's created. * Avoid unnecessary CultureInfo access in derived TextWriters SyncTextWriter already overrides FormatProvider, in which case the t.FormatProvider passed to the base will never be used, so this call is incurring a virtual dispatch for no benefit. And NullTextWriter needn't access InvariantCulture and force it into existence if it isn't yet, as the formatting should never actually be used, and if it is, its FormatProvider override can supply the culture. * Avoid allocating AssemblyLoadContext's dictionary if no direct interaction with ALC AssemblyLoadContext.OnProcessExit gets called by EventSource, which in turn forces s_allContexts into existence in order to lock on it in order to enumerate all active contexts, and if there's been no interaction with AssemblyLoadContext, there won't be any to enumerate. So delay allocate the object. * Address PR feedback * Call EventListener.DisposeOnShutdown from AppContext.OnProcessExit Avoids the need to register with AppContext.ProcessExit, avoiding an EventHandler allocation, and avoids the need in the common case to fire AppContext.ProcessExit, which in turn avoids allocating an AppDomain and EventArgs if they weren't otherwise created, plus it avoids the delegate invocation. * Update src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs --- diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ComparerHelpers.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ComparerHelpers.cs index 1fb5467..6812a6c 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ComparerHelpers.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ComparerHelpers.cs @@ -120,28 +120,33 @@ namespace System.Collections.Generic object? result = null; var runtimeType = (RuntimeType)type; - // Specialize for byte so Array.IndexOf is faster. if (type == typeof(byte)) { + // Specialize for byte so Array.IndexOf is faster. result = new ByteEqualityComparer(); } - // If T implements IEquatable return a GenericEqualityComparer + else if (type == typeof(string)) + { + // Specialize for string, as EqualityComparer.Default is on the startup path + result = new GenericEqualityComparer(); + } else if (type.IsAssignableTo(typeof(IEquatable<>).MakeGenericType(type))) { - result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericEqualityComparer), runtimeType); + // If T implements IEquatable return a GenericEqualityComparer + result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericEqualityComparer), runtimeType); } - // Nullable does not implement IEquatable directly because that would add an extra interface call per comparison. - // Instead, it relies on EqualityComparer.Default to specialize for nullables and do the lifted comparisons if T implements IEquatable. else if (type.IsGenericType) { + // Nullable does not implement IEquatable directly because that would add an extra interface call per comparison. + // Instead, it relies on EqualityComparer.Default to specialize for nullables and do the lifted comparisons if T implements IEquatable. if (type.GetGenericTypeDefinition() == typeof(Nullable<>)) { result = TryCreateNullableEqualityComparer(runtimeType); } } - // The equality comparer for enums is specialized to avoid boxing. else if (type.IsEnum) { + // The equality comparer for enums is specialized to avoid boxing. result = TryCreateEnumEqualityComparer(runtimeType); } diff --git a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.EventSetInformation.cs b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.EventSetInformation.cs index d727076..a623021 100644 --- a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.EventSetInformation.cs +++ b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.EventSetInformation.cs @@ -12,6 +12,6 @@ internal static partial class Interop long registrationHandle, EVENT_INFO_CLASS informationClass, void* eventInformation, - int informationLength); + uint informationLength); } } diff --git a/src/libraries/Common/src/System/Text/EncodingHelper.Windows.cs b/src/libraries/Common/src/System/Text/EncodingHelper.Windows.cs index acd0482..7cd16bc 100644 --- a/src/libraries/Common/src/System/Text/EncodingHelper.Windows.cs +++ b/src/libraries/Common/src/System/Text/EncodingHelper.Windows.cs @@ -1,14 +1,20 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Text; +using System.Diagnostics; namespace System.Text { // If we find issues with this or if more libraries need this behavior we will revisit the solution. internal static partial class EncodingHelper { + /// Hardcoded Encoding.UTF8.CodePage to avoid accessing Encoding.Unicode and forcing it into existence unnecessarily. + private const int Utf8CodePage = 65001; + +#if DEBUG + static EncodingHelper() => Debug.Assert(Utf8CodePage == Encoding.UTF8.CodePage); +#endif + // Since only a minimum set of encodings are available by default, // Console encoding might not be available and require provider registering. // To avoid encoding exception in Console APIs we fallback to OSEncoding. @@ -27,12 +33,12 @@ namespace System.Text { int defaultEncCodePage = Encoding.GetEncoding(0).CodePage; - if ((defaultEncCodePage == codepage) || defaultEncCodePage != Encoding.UTF8.CodePage) + if (defaultEncCodePage == codepage || defaultEncCodePage != Utf8CodePage) { return Encoding.GetEncoding(codepage); } - if (codepage != Encoding.UTF8.CodePage) + if (codepage != Utf8CodePage) { return new OSEncoding(codepage); } diff --git a/src/libraries/System.Console/src/System/ConsolePal.Windows.cs b/src/libraries/System.Console/src/System/ConsolePal.Windows.cs index 513e281..76edfe8 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Windows.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Windows.cs @@ -11,6 +11,13 @@ namespace System // Provides Windows-based support for System.Console. internal static class ConsolePal { + /// Hardcoded Encoding.Unicode.CodePage to avoid accessing Encoding.Unicode and forcing it into existence unnecessarily. + private const int UnicodeCodePage = 1200; + +#if DEBUG + static ConsolePal() => Debug.Assert(UnicodeCodePage == Encoding.Unicode.CodePage); +#endif + private static IntPtr InvalidHandleValue => new IntPtr(-1); /// Ensures that the console has been initialized for use. @@ -29,19 +36,19 @@ namespace System GetStandardFile( Interop.Kernel32.HandleTypes.STD_INPUT_HANDLE, FileAccess.Read, - useFileAPIs: Console.InputEncoding.CodePage != Encoding.Unicode.CodePage || Console.IsInputRedirected); + useFileAPIs: Console.InputEncoding.CodePage != UnicodeCodePage || Console.IsInputRedirected); public static Stream OpenStandardOutput() => GetStandardFile( Interop.Kernel32.HandleTypes.STD_OUTPUT_HANDLE, FileAccess.Write, - useFileAPIs: Console.OutputEncoding.CodePage != Encoding.Unicode.CodePage || Console.IsOutputRedirected); + useFileAPIs: Console.OutputEncoding.CodePage != UnicodeCodePage || Console.IsOutputRedirected); public static Stream OpenStandardError() => GetStandardFile( Interop.Kernel32.HandleTypes.STD_ERROR_HANDLE, FileAccess.Write, - useFileAPIs: Console.OutputEncoding.CodePage != Encoding.Unicode.CodePage || Console.IsErrorRedirected); + useFileAPIs: Console.OutputEncoding.CodePage != UnicodeCodePage || Console.IsErrorRedirected); private static IntPtr InputHandle => Interop.Kernel32.GetStdHandle(Interop.Kernel32.HandleTypes.STD_INPUT_HANDLE); @@ -99,7 +106,7 @@ namespace System public static void SetConsoleInputEncoding(Encoding enc) { - if (enc.CodePage != Encoding.Unicode.CodePage) + if (enc.CodePage != UnicodeCodePage) { if (!Interop.Kernel32.SetConsoleCP(enc.CodePage)) throw Win32Marshal.GetExceptionForWin32Error(Marshal.GetLastWin32Error()); @@ -113,7 +120,7 @@ namespace System public static void SetConsoleOutputEncoding(Encoding enc) { - if (enc.CodePage != Encoding.Unicode.CodePage) + if (enc.CodePage != UnicodeCodePage) { if (!Interop.Kernel32.SetConsoleOutputCP(enc.CodePage)) throw Win32Marshal.GetExceptionForWin32Error(Marshal.GetLastWin32Error()); diff --git a/src/libraries/System.Private.CoreLib/src/System/AppContext.cs b/src/libraries/System.Private.CoreLib/src/System/AppContext.cs index 8c187fa..35e45f4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/AppContext.cs +++ b/src/libraries/System.Private.CoreLib/src/System/AppContext.cs @@ -3,7 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; -using System.IO; +using System.Diagnostics.Tracing; using System.Reflection; using System.Runtime.ExceptionServices; using System.Runtime.Loader; @@ -64,14 +64,18 @@ namespace System #pragma warning disable CS0067 // events raised by the VM public static event UnhandledExceptionEventHandler? UnhandledException; - public static event System.EventHandler? FirstChanceException; + public static event EventHandler? FirstChanceException; #pragma warning restore CS0067 - public static event System.EventHandler? ProcessExit; + public static event EventHandler? ProcessExit; internal static void OnProcessExit() { AssemblyLoadContext.OnProcessExit(); + if (EventSource.IsSupported) + { + EventListener.DisposeOnShutdown(); + } ProcessExit?.Invoke(AppDomain.CurrentDomain, EventArgs.Empty); } diff --git a/src/libraries/System.Private.CoreLib/src/System/AppDomain.cs b/src/libraries/System.Private.CoreLib/src/System/AppDomain.cs index 06c6410..5101f42 100644 --- a/src/libraries/System.Private.CoreLib/src/System/AppDomain.cs +++ b/src/libraries/System.Private.CoreLib/src/System/AppDomain.cs @@ -20,7 +20,6 @@ namespace System public sealed partial class AppDomain : MarshalByRefObject { private static readonly AppDomain s_domain = new AppDomain(); - private readonly object _forLock = new object(); private IPrincipal? _defaultPrincipal; private PrincipalPolicy _principalPolicy = PrincipalPolicy.NoPrincipal; private Func? s_getWindowsPrincipal; @@ -272,14 +271,10 @@ namespace System throw new ArgumentNullException(nameof(principal)); } - lock (_forLock) + // Set the principal while checking it has not been set previously. + if (Interlocked.CompareExchange(ref _defaultPrincipal, principal, null) is not null) { - // Check that principal has not been set previously. - if (_defaultPrincipal != null) - { - throw new SystemException(SR.AppDomain_Policy_PrincipalTwice); - } - _defaultPrincipal = principal; + throw new SystemException(SR.AppDomain_Policy_PrincipalTwice); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs index 444b984..99fd304 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs @@ -1235,7 +1235,7 @@ namespace System.Diagnostics.Tracing internal unsafe int SetInformation( Interop.Advapi32.EVENT_INFO_CLASS eventInfoClass, - IntPtr data, + void* data, uint dataSize) { int status = Interop.Errors.ERROR_NOT_SUPPORTED; @@ -1247,8 +1247,8 @@ namespace System.Diagnostics.Tracing status = Interop.Advapi32.EventSetInformation( m_regHandle, eventInfoClass, - (void*)data, - (int)dataSize); + data, + dataSize); } catch (TypeLoadException) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs index 2492b01..e3d0c2b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs @@ -229,7 +229,7 @@ namespace System.Diagnostics.Tracing #pragma warning restore CA1823 #endif //FEATURE_EVENTSOURCE_XPLAT - private static bool IsSupported { get; } = InitializeIsSupported(); + internal static bool IsSupported { get; } = InitializeIsSupported(); private static bool InitializeIsSupported() => AppContext.TryGetSwitch("System.Diagnostics.Tracing.EventSource.IsSupported", out bool isSupported) ? isSupported : true; @@ -1508,17 +1508,13 @@ namespace System.Diagnostics.Tracing if (this.Name != "System.Diagnostics.Eventing.FrameworkEventSource" || Environment.IsWindows8OrAbove) #endif { - int setInformationResult; - System.Runtime.InteropServices.GCHandle metadataHandle = - System.Runtime.InteropServices.GCHandle.Alloc(this.providerMetadata, System.Runtime.InteropServices.GCHandleType.Pinned); - IntPtr providerMetadata = metadataHandle.AddrOfPinnedObject(); - - setInformationResult = m_etwProvider.SetInformation( - Interop.Advapi32.EVENT_INFO_CLASS.SetTraits, - providerMetadata, - (uint)this.providerMetadata.Length); - - metadataHandle.Free(); + fixed (byte* providerMetadata = this.providerMetadata) + { + m_etwProvider.SetInformation( + Interop.Advapi32.EVENT_INFO_CLASS.SetTraits, + providerMetadata, + (uint)this.providerMetadata.Length); + } } #endif // TARGET_WINDOWS #endif // FEATURE_MANAGED_ETW @@ -2241,6 +2237,11 @@ namespace System.Diagnostics.Tracing { if (m_writeEventStringEventHandle == IntPtr.Zero) { + if (m_createEventLock is null) + { + Interlocked.CompareExchange(ref m_createEventLock, new object(), null); + } + lock (m_createEventLock) { if (m_writeEventStringEventHandle == IntPtr.Zero) @@ -3771,7 +3772,7 @@ namespace System.Diagnostics.Tracing private volatile OverideEventProvider m_etwProvider = null!; // This hooks up ETW commands to our 'OnEventCommand' callback #endif #if FEATURE_PERFTRACING - private object m_createEventLock = new object(); + private object? m_createEventLock; private IntPtr m_writeEventStringEventHandle = IntPtr.Zero; private volatile OverideEventProvider m_eventPipeProvider = null!; #endif @@ -4116,18 +4117,17 @@ namespace System.Diagnostics.Tracing { lock (EventListenersLock) { - s_EventSources ??= new List>(2); + Debug.Assert(s_EventSources != null); +#if ES_BUILD_STANDALONE + // netcoreapp build calls DisposeOnShutdown directly from AppContext.OnProcessExit if (!s_EventSourceShutdownRegistered) { s_EventSourceShutdownRegistered = true; -#if ES_BUILD_STANDALONE AppDomain.CurrentDomain.ProcessExit += DisposeOnShutdown; AppDomain.CurrentDomain.DomainUnload += DisposeOnShutdown; -#else - AppContext.ProcessExit += DisposeOnShutdown; -#endif } +#endif // Periodically search the list for existing entries to reuse, this avoids // unbounded memory use if we keep recycling eventSources (an unlikely thing). @@ -4184,8 +4184,14 @@ namespace System.Diagnostics.Tracing // such callbacks on process shutdown or appdomain so that unmanaged code will never // do this. This is what this callback is for. // See bug 724140 for more +#if ES_BUILD_STANDALONE private static void DisposeOnShutdown(object? sender, EventArgs e) +#else + internal static void DisposeOnShutdown() +#endif { + Debug.Assert(EventSource.IsSupported); + lock (EventListenersLock) { Debug.Assert(s_EventSources != null); @@ -4420,10 +4426,12 @@ namespace System.Diagnostics.Tracing private static bool s_ConnectingEventSourcesAndListener; #endif +#if ES_BUILD_STANDALONE /// /// Used to register AD/Process shutdown callbacks. /// private static bool s_EventSourceShutdownRegistered; +#endif #endregion } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs b/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs index d969ee2..65e9428 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs @@ -703,10 +703,12 @@ namespace System.IO private sealed class NullTextWriter : TextWriter { - internal NullTextWriter() : base(CultureInfo.InvariantCulture) + internal NullTextWriter() { } + public override IFormatProvider FormatProvider => CultureInfo.InvariantCulture; + public override Encoding Encoding => Encoding.Unicode; public override void Write(char[] buffer, int index, int count) @@ -748,7 +750,7 @@ namespace System.IO { private readonly TextWriter _out; - internal SyncTextWriter(TextWriter t) : base(t.FormatProvider) + internal SyncTextWriter(TextWriter t) { _out = t; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs index 3a14f9d..6e6fec0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs @@ -29,9 +29,15 @@ namespace System.Runtime.Loader Unloading } - private static readonly Dictionary> s_allContexts = new Dictionary>(); + private static volatile Dictionary>? s_allContexts; private static long s_nextId; + [MemberNotNull(nameof(s_allContexts))] + private static Dictionary> AllContexts => + s_allContexts ?? + Interlocked.CompareExchange(ref s_allContexts, new Dictionary>(), null) ?? + s_allContexts; + #region private data members // If you modify any of these fields, you must also update the // AssemblyLoadContextBaseObject structure in object.h @@ -96,10 +102,11 @@ namespace System.Runtime.Loader _nativeAssemblyLoadContext = InitializeAssemblyLoadContext(thisHandlePtr, representsTPALoadContext, isCollectible); // Add this instance to the list of alive ALC - lock (s_allContexts) + Dictionary> allContexts = AllContexts; + lock (allContexts) { _id = s_nextId++; - s_allContexts.Add(_id, new WeakReference(this, true)); + allContexts.Add(_id, new WeakReference(this, true)); } } @@ -142,9 +149,10 @@ namespace System.Runtime.Loader _state = InternalState.Unloading; } - lock (s_allContexts) + Dictionary> allContexts = AllContexts; + lock (allContexts) { - s_allContexts.Remove(_id); + allContexts.Remove(_id); } } @@ -239,16 +247,24 @@ namespace System.Runtime.Loader { get { - _ = AssemblyLoadContext.Default; // Ensure default is initialized + _ = Default; // Ensure default is initialized + + Dictionary>? allContexts = s_allContexts; + Debug.Assert(allContexts != null, "Creating the default context should have initialized the contexts collection."); - List>? alcList = null; - lock (s_allContexts) + WeakReference[] alcSnapshot; + lock (allContexts) { // To make this thread safe we need a quick snapshot while locked - alcList = new List>(s_allContexts.Values); + alcSnapshot = new WeakReference[allContexts.Count]; + int pos = 0; + foreach (KeyValuePair> item in allContexts) + { + alcSnapshot[pos++] = item.Value; + } } - foreach (WeakReference weakAlc in alcList) + foreach (WeakReference weakAlc in alcSnapshot) { if (weakAlc.TryGetTarget(out AssemblyLoadContext? alc)) { @@ -428,9 +444,16 @@ namespace System.Runtime.Loader internal static void OnProcessExit() { - lock (s_allContexts) + Dictionary>? allContexts = s_allContexts; + if (allContexts is null) + { + // If s_allContexts was never initialized, there are no contexts for which to raise an unload event. + return; + } + + lock (allContexts) { - foreach (KeyValuePair> alcAlive in s_allContexts) + foreach (KeyValuePair> alcAlive in allContexts) { if (alcAlive.Value.TryGetTarget(out AssemblyLoadContext? alc)) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/EncodingProvider.cs b/src/libraries/System.Private.CoreLib/src/System/Text/EncodingProvider.cs index e39e75b..8857f0f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/EncodingProvider.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/EncodingProvider.cs @@ -2,11 +2,14 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Threading; namespace System.Text { public abstract class EncodingProvider { + private static volatile EncodingProvider[]? s_providers; + public EncodingProvider() { } public abstract Encoding? GetEncoding(string name); public abstract Encoding? GetEncoding(int codepage); @@ -42,26 +45,38 @@ namespace System.Text internal static void AddProvider(EncodingProvider provider) { - if (provider == null) + if (provider is null) + { throw new ArgumentNullException(nameof(provider)); + } + + // Few providers are added in a typical app (typically just CodePagesEncodingProvider.Instance), and when they are, + // they're generally not added concurrently. So use an optimistic concurrency scheme rather than paying for a lock + // object allocation on the startup path. + + if (s_providers is null && + Interlocked.CompareExchange(ref s_providers, new EncodingProvider[1] { provider }, null) is null) + { + return; + } - lock (s_InternalSyncObject) + while (true) { - if (s_providers == null) + EncodingProvider[] providers = s_providers; + + if (Array.IndexOf(providers, provider) >= 0) { - s_providers = new EncodingProvider[1] { provider }; return; } - if (Array.IndexOf(s_providers, provider) >= 0) + var newProviders = new EncodingProvider[providers.Length + 1]; + Array.Copy(providers, newProviders, providers.Length); + providers[^1] = provider; + + if (Interlocked.CompareExchange(ref s_providers, newProviders, providers) == providers) { return; } - - EncodingProvider[] providers = new EncodingProvider[s_providers.Length + 1]; - Array.Copy(s_providers, providers, s_providers.Length); - providers[^1] = provider; - s_providers = providers; } } @@ -151,8 +166,5 @@ namespace System.Text return null; } - - private static readonly object s_InternalSyncObject = new object(); - private static volatile EncodingProvider[]? s_providers; } }