From ad418f66d55ae9430ae1088c9c90ab528dce4f96 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 22 Jun 2023 00:10:07 -0400 Subject: [PATCH] Fix contention in LoggerFactory.CreateLogger Some ASP.NET code paths end up calling this _a lot_, and it takes a lock on every call, even though the vast majority are satisifed by the cache and require no mutation. We can use a ConcurrentDictionary instead of a Dictionary, with double-checked locking. Nothing is ever removed from or overwritten in the dictionary, so there's no problem doing the hot-path read outside of the lock; we still do the mutation inside of the lock so that all of the mutation work is performed atomically and synchronized with the actions from AddProvider and RefreshFilter. --- .../src/LoggerFactory.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs b/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs index bff52609d62bca..305860f6e91616 100644 --- a/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs +++ b/src/libraries/Microsoft.Extensions.Logging/src/LoggerFactory.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using Microsoft.Extensions.DependencyInjection; @@ -14,7 +15,7 @@ namespace Microsoft.Extensions.Logging /// public class LoggerFactory : ILoggerFactory { - private readonly Dictionary _loggers = new Dictionary(StringComparer.Ordinal); + private readonly ConcurrentDictionary _loggers = new ConcurrentDictionary(StringComparer.Ordinal); private readonly List _providerRegistrations = new List(); private readonly object _sync = new object(); private volatile bool _disposed; @@ -138,19 +139,22 @@ public ILogger CreateLogger(string categoryName) throw new ObjectDisposedException(nameof(LoggerFactory)); } - lock (_sync) + if (!_loggers.TryGetValue(categoryName, out Logger? logger)) { - if (!_loggers.TryGetValue(categoryName, out Logger? logger)) + lock (_sync) { - logger = new Logger(categoryName, CreateLoggers(categoryName)); + if (!_loggers.TryGetValue(categoryName, out logger)) + { + logger = new Logger(categoryName, CreateLoggers(categoryName)); - (logger.MessageLoggers, logger.ScopeLoggers) = ApplyFilters(logger.Loggers); + (logger.MessageLoggers, logger.ScopeLoggers) = ApplyFilters(logger.Loggers); - _loggers[categoryName] = logger; + _loggers[categoryName] = logger; + } } - - return logger; } + + return logger; } ///