From 1be05ec05837be42c6449a67f82783ebbb5d3b90 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 9 Nov 2020 14:48:40 -0500 Subject: [PATCH 01/11] Specialize `EqualityComparer.Default` Removes ~80 allocations at startup. --- .../System/Collections/Generic/ComparerHelpers.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) 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 1fb54677d0c5a3..c6d3b25759ff0a 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 @@ internal static object CreateDefaultEqualityComparer(Type type) 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))) { + // 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); } From d5b113621c6368ff7604506b71e5879c615ca7fc Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 9 Nov 2020 16:56:33 -0500 Subject: [PATCH 02/11] Avoid loading Encoding.Unicode just for CodePage --- .../src/System/Text/EncodingHelper.Windows.cs | 14 ++++++++++---- .../src/System/ConsolePal.Windows.cs | 17 ++++++++++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/src/System/Text/EncodingHelper.Windows.cs b/src/libraries/Common/src/System/Text/EncodingHelper.Windows.cs index acd04824ee876d..7cd16bc021d4d3 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 @@ internal static Encoding GetSupportedConsoleEncoding(int codepage) { 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 513e281eac7529..76edfe8512e858 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 @@ public static Stream OpenStandardInput() => 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 @@ public static Encoding InputEncoding 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 @@ public static Encoding OutputEncoding 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()); From 9dcee3ed7ebf53999d8a03974700d901e1761d4c Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 9 Nov 2020 18:04:26 -0500 Subject: [PATCH 03/11] Use fixed instead of GCHandle.Alloc in EventSource.Initialize --- .../Advapi32/Interop.EventSetInformation.cs | 2 +- .../Diagnostics/Tracing/EventProvider.cs | 6 +++--- .../System/Diagnostics/Tracing/EventSource.cs | 18 +++++++----------- 3 files changed, 11 insertions(+), 15 deletions(-) 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 d7270765157a43..a6230216aee027 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 extern unsafe int EventSetInformation( long registrationHandle, EVENT_INFO_CLASS informationClass, void* eventInformation, - int informationLength); + uint informationLength); } } 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 444b9847e2025e..99fd3045509211 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 @@ private void EventUnregister(long registrationHandle) => 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 @@ internal unsafe int SetInformation( 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 2492b0161b4d33..d49554639eb2e7 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 @@ -1508,17 +1508,13 @@ private unsafe void Initialize(Guid eventSourceGuid, string eventSourceName, str 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 From c416ae56397807b6b006953fe82cdbc3c027c649 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 9 Nov 2020 20:37:20 -0500 Subject: [PATCH 04/11] Remove lock / lock object from EncodingProvider.AddProvider --- .../src/System/Text/EncodingProvider.cs | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) 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 e39e75b52551c0..8857f0fa713bf0 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 @@ public EncodingProvider() { } 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 @@ internal static void AddProvider(EncodingProvider provider) return null; } - - private static readonly object s_InternalSyncObject = new object(); - private static volatile EncodingProvider[]? s_providers; } } From 0d958d4f9308f6bd982637725c7d1c0e4089bd69 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 9 Nov 2020 20:42:27 -0500 Subject: [PATCH 05/11] Remove lock object from AppDomain.cs --- .../System.Private.CoreLib/src/System/AppDomain.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/AppDomain.cs b/src/libraries/System.Private.CoreLib/src/System/AppDomain.cs index 06c64105edd097..5101f42b1a781c 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 @@ public void SetThreadPrincipal(IPrincipal principal) 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); } } From 9bb17c5dbfc1b938b71cf24a623ff4f94531c13f Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 9 Nov 2020 22:07:37 -0500 Subject: [PATCH 06/11] 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. --- .../src/System/Diagnostics/Tracing/EventSource.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 d49554639eb2e7..56025c31e99da2 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 @@ -2237,6 +2237,11 @@ private unsafe void WriteEventString(string msgString) { 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) @@ -3767,7 +3772,7 @@ private bool SelfDescribingEvents 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 From e6d806ea8061a6d2c3257e6d80f797e7551cfa7f Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 10 Nov 2020 09:44:58 -0500 Subject: [PATCH 07/11] 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. --- .../System.Private.CoreLib/src/System/IO/TextWriter.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 d969ee23a919c7..65e94282e5d240 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 @@ public virtual Task FlushAsync() 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 @@ internal sealed class SyncTextWriter : TextWriter, IDisposable { private readonly TextWriter _out; - internal SyncTextWriter(TextWriter t) : base(t.FormatProvider) + internal SyncTextWriter(TextWriter t) { _out = t; } From 74b57c2d1c1458a662befeebea6ac9761cf407ec Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 10 Nov 2020 10:23:33 -0500 Subject: [PATCH 08/11] 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. --- .../Runtime/Loader/AssemblyLoadContext.cs | 47 ++++++++++++++----- 1 file changed, 35 insertions(+), 12 deletions(-) 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 3a14f9d0a9cf21..6e6fec01d8cca7 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 @@ private enum InternalState 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 @@ private protected AssemblyLoadContext(bool representsTPALoadContext, bool isColl _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 @@ private void InitiateUnload() _state = InternalState.Unloading; } - lock (s_allContexts) + Dictionary> allContexts = AllContexts; + lock (allContexts) { - s_allContexts.Remove(_id); + allContexts.Remove(_id); } } @@ -239,16 +247,24 @@ public static IEnumerable All { 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 @@ public void Unload() 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)) { From 6f68d3757082398f56d2b0f2f5c430b600e2e3d4 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 10 Nov 2020 20:54:29 -0500 Subject: [PATCH 09/11] Address PR feedback --- .../src/System/Collections/Generic/ComparerHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c6d3b25759ff0a..6812a6c92faf26 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 @@ -133,7 +133,7 @@ internal static object CreateDefaultEqualityComparer(Type type) else if (type.IsAssignableTo(typeof(IEquatable<>).MakeGenericType(type))) { // If T implements IEquatable return a GenericEqualityComparer - result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericEqualityComparer), runtimeType); + result = CreateInstanceForAnotherGenericParameter((RuntimeType)typeof(GenericEqualityComparer), runtimeType); } else if (type.IsGenericType) { From f0c3105b6f331a821dab2e4ef082363e4040e0bb Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 10 Nov 2020 21:05:25 -0500 Subject: [PATCH 10/11] 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. --- .../src/System/AppContext.cs | 10 ++++++--- .../System/Diagnostics/Tracing/EventSource.cs | 22 ++++++++++++++----- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/AppContext.cs b/src/libraries/System.Private.CoreLib/src/System/AppContext.cs index 8c187fa53e6290..35e45f472bb8b1 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 @@ public static void SetData(string name, object? data) #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/Diagnostics/Tracing/EventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs index 56025c31e99da2..b33c0e5b7a4c95 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 @@ public partial class EventSource : IDisposable #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; @@ -4117,18 +4117,17 @@ internal static void AddEventSource(EventSource newEventSource) { 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). @@ -4185,8 +4184,17 @@ internal static void AddEventSource(EventSource newEventSource) // 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 { + if (!EventSource.IsSupported) + { + return; + } + lock (EventListenersLock) { Debug.Assert(s_EventSources != null); @@ -4421,10 +4429,12 @@ private void CallBackForExistingEventSources(bool addToListenersList, EventHandl private static bool s_ConnectingEventSourcesAndListener; #endif +#if ES_BUILD_STANDALONE /// /// Used to register AD/Process shutdown callbacks. /// private static bool s_EventSourceShutdownRegistered; +#endif #endregion } From b68fdaff07c9e537798d572e2381da330bcbe271 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 10 Nov 2020 21:21:05 -0500 Subject: [PATCH 11/11] Update src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs --- .../src/System/Diagnostics/Tracing/EventSource.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) 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 b33c0e5b7a4c95..e3d0c2b4a9e752 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 @@ -4190,10 +4190,7 @@ private static void DisposeOnShutdown(object? sender, EventArgs e) internal static void DisposeOnShutdown() #endif { - if (!EventSource.IsSupported) - { - return; - } + Debug.Assert(EventSource.IsSupported); lock (EventListenersLock) {