Remove some allocation at "hello world" startup (#44469)
authorStephen Toub <stoub@microsoft.com>
Wed, 11 Nov 2020 14:15:06 +0000 (09:15 -0500)
committerGitHub <noreply@github.com>
Wed, 11 Nov 2020 14:15:06 +0000 (09:15 -0500)
* Specialize `EqualityComparer<string>.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

src/coreclr/src/System.Private.CoreLib/src/System/Collections/Generic/ComparerHelpers.cs
src/libraries/Common/src/Interop/Windows/Advapi32/Interop.EventSetInformation.cs
src/libraries/Common/src/System/Text/EncodingHelper.Windows.cs
src/libraries/System.Console/src/System/ConsolePal.Windows.cs
src/libraries/System.Private.CoreLib/src/System/AppContext.cs
src/libraries/System.Private.CoreLib/src/System/AppDomain.cs
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventProvider.cs
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs
src/libraries/System.Private.CoreLib/src/System/IO/TextWriter.cs
src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.cs
src/libraries/System.Private.CoreLib/src/System/Text/EncodingProvider.cs

index 1fb5467..6812a6c 100644 (file)
@@ -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<T> return a GenericEqualityComparer<T>
+            else if (type == typeof(string))
+            {
+                // Specialize for string, as EqualityComparer<string>.Default is on the startup path
+                result = new GenericEqualityComparer<string>();
+            }
             else if (type.IsAssignableTo(typeof(IEquatable<>).MakeGenericType(type)))
             {
-                result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericEqualityComparer<int>), runtimeType);
+                // If T implements IEquatable<T> return a GenericEqualityComparer<T>
+                result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericEqualityComparer<string>), runtimeType);
             }
-            // Nullable does not implement IEquatable<T?> directly because that would add an extra interface call per comparison.
-            // Instead, it relies on EqualityComparer<T?>.Default to specialize for nullables and do the lifted comparisons if T implements IEquatable.
             else if (type.IsGenericType)
             {
+                // Nullable does not implement IEquatable<T?> directly because that would add an extra interface call per comparison.
+                // Instead, it relies on EqualityComparer<T?>.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);
             }
 
index d727076..a623021 100644 (file)
@@ -12,6 +12,6 @@ internal static partial class Interop
             long registrationHandle,
             EVENT_INFO_CLASS informationClass,
             void* eventInformation,
-            int informationLength);
+            uint informationLength);
     }
 }
index acd0482..7cd16bc 100644 (file)
@@ -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
     {
+        /// <summary>Hardcoded Encoding.UTF8.CodePage to avoid accessing Encoding.Unicode and forcing it into existence unnecessarily.</summary>
+        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);
             }
index 513e281..76edfe8 100644 (file)
@@ -11,6 +11,13 @@ namespace System
     // Provides Windows-based support for System.Console.
     internal static class ConsolePal
     {
+        /// <summary>Hardcoded Encoding.Unicode.CodePage to avoid accessing Encoding.Unicode and forcing it into existence unnecessarily.</summary>
+        private const int UnicodeCodePage = 1200;
+
+#if DEBUG
+        static ConsolePal() => Debug.Assert(UnicodeCodePage == Encoding.Unicode.CodePage);
+#endif
+
         private static IntPtr InvalidHandleValue => new IntPtr(-1);
 
         /// <summary>Ensures that the console has been initialized for use.</summary>
@@ -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());
index 8c187fa..35e45f4 100644 (file)
@@ -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<FirstChanceExceptionEventArgs>? FirstChanceException;
+        public static event EventHandler<FirstChanceExceptionEventArgs>? 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);
         }
index 06c6410..5101f42 100644 (file)
@@ -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<IPrincipal>? 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);
             }
         }
 
index 444b984..99fd304 100644 (file)
@@ -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)
                 {
index 2492b01..e3d0c2b 100644 (file)
@@ -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<WeakReference<EventSource>>(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
         /// <summary>
         /// Used to register AD/Process shutdown callbacks.
         /// </summary>
         private static bool s_EventSourceShutdownRegistered;
+#endif
 #endregion
     }
 
index d969ee2..65e9428 100644 (file)
@@ -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;
             }
index 3a14f9d..6e6fec0 100644 (file)
@@ -29,9 +29,15 @@ namespace System.Runtime.Loader
             Unloading
         }
 
-        private static readonly Dictionary<long, WeakReference<AssemblyLoadContext>> s_allContexts = new Dictionary<long, WeakReference<AssemblyLoadContext>>();
+        private static volatile Dictionary<long, WeakReference<AssemblyLoadContext>>? s_allContexts;
         private static long s_nextId;
 
+        [MemberNotNull(nameof(s_allContexts))]
+        private static Dictionary<long, WeakReference<AssemblyLoadContext>> AllContexts =>
+            s_allContexts ??
+            Interlocked.CompareExchange(ref s_allContexts, new Dictionary<long, WeakReference<AssemblyLoadContext>>(), 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<long, WeakReference<AssemblyLoadContext>> allContexts = AllContexts;
+            lock (allContexts)
             {
                 _id = s_nextId++;
-                s_allContexts.Add(_id, new WeakReference<AssemblyLoadContext>(this, true));
+                allContexts.Add(_id, new WeakReference<AssemblyLoadContext>(this, true));
             }
         }
 
@@ -142,9 +149,10 @@ namespace System.Runtime.Loader
                 _state = InternalState.Unloading;
             }
 
-            lock (s_allContexts)
+            Dictionary<long, WeakReference<AssemblyLoadContext>> 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<long, WeakReference<AssemblyLoadContext>>? allContexts = s_allContexts;
+                Debug.Assert(allContexts != null, "Creating the default context should have initialized the contexts collection.");
 
-                List<WeakReference<AssemblyLoadContext>>? alcList = null;
-                lock (s_allContexts)
+                WeakReference<AssemblyLoadContext>[] alcSnapshot;
+                lock (allContexts)
                 {
                     // To make this thread safe we need a quick snapshot while locked
-                    alcList = new List<WeakReference<AssemblyLoadContext>>(s_allContexts.Values);
+                    alcSnapshot = new WeakReference<AssemblyLoadContext>[allContexts.Count];
+                    int pos = 0;
+                    foreach (KeyValuePair<long, WeakReference<AssemblyLoadContext>> item in allContexts)
+                    {
+                        alcSnapshot[pos++] = item.Value;
+                    }
                 }
 
-                foreach (WeakReference<AssemblyLoadContext> weakAlc in alcList)
+                foreach (WeakReference<AssemblyLoadContext> 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<long, WeakReference<AssemblyLoadContext>>? 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<long, WeakReference<AssemblyLoadContext>> alcAlive in s_allContexts)
+                foreach (KeyValuePair<long, WeakReference<AssemblyLoadContext>> alcAlive in allContexts)
                 {
                     if (alcAlive.Value.TryGetTarget(out AssemblyLoadContext? alc))
                     {
index e39e75b..8857f0f 100644 (file)
@@ -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;
     }
 }