From 5e9d3980c25272fe14bed7e3515e87f00e05f646 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 9 Feb 2017 08:57:13 -0800 Subject: [PATCH] Option 2: Just cap --- .../Internal/ConcurrentLruCache.cs | 237 ------------------ .../Internal/FormattedLogValues.cs | 16 +- 2 files changed, 14 insertions(+), 239 deletions(-) delete mode 100644 src/Microsoft.Extensions.Logging.Abstractions/Internal/ConcurrentLruCache.cs diff --git a/src/Microsoft.Extensions.Logging.Abstractions/Internal/ConcurrentLruCache.cs b/src/Microsoft.Extensions.Logging.Abstractions/Internal/ConcurrentLruCache.cs deleted file mode 100644 index cd74d12b..00000000 --- a/src/Microsoft.Extensions.Logging.Abstractions/Internal/ConcurrentLruCache.cs +++ /dev/null @@ -1,237 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Diagnostics; - -namespace Microsoft.Extensions.Logging.Internal -{ - /// - /// Cache with a fixed size that evicts the least recently used members. - /// Thread-safe. - /// - internal class ConcurrentLruCache - { - private readonly int _capacity; - - private struct CacheValue - { - public V Value; - public LinkedListNode Node; - } - - private readonly Dictionary _cache; - private readonly LinkedList _nodeList; - // This is a naive course-grained lock, it can probably be optimized - private readonly object _lockObject = new object(); - - public ConcurrentLruCache(int capacity) - { - if (capacity <= 0) - { - throw new ArgumentOutOfRangeException(nameof(capacity)); - } - _capacity = capacity; - _cache = new Dictionary(capacity); - _nodeList = new LinkedList(); - } - - /// - /// Create cache from an array. The cache capacity will be the size - /// of the array. All elements of the array will be added to the - /// cache. If any duplicate keys are found in the array a - /// will be thrown. - /// - public ConcurrentLruCache(KeyValuePair[] array) - : this(array.Length) - { - foreach (var kvp in array) - { - this.UnsafeAdd(kvp.Key, kvp.Value, true); - } - } - - /// - /// For testing. Very expensive. - /// - internal IEnumerable> TestingEnumerable - { - get - { - lock (_lockObject) - { - var copy = new KeyValuePair[_cache.Count]; - int index = 0; - foreach (K key in _nodeList) - { - copy[index++] = new KeyValuePair(key, - _cache[key].Value); - } - return copy; - } - } - } - - public void Add(K key, V value) - { - lock (_lockObject) - { - UnsafeAdd(key, value, true); - } - } - - private void MoveNodeToTop(LinkedListNode node) - { - if (!object.ReferenceEquals(_nodeList.First, node)) - { - _nodeList.Remove(node); - _nodeList.AddFirst(node); - } - } - - /// - /// Expects non-empty cache. Does not lock. - /// - private void UnsafeEvictLastNode() - { - Debug.Assert(_capacity > 0); - var lastNode = _nodeList.Last; - _nodeList.Remove(lastNode); - _cache.Remove(lastNode.Value); - } - - private void UnsafeAddNodeToTop(K key, V value) - { - var node = new LinkedListNode(key); - _cache.Add(key, new CacheValue { Node = node, Value = value }); - _nodeList.AddFirst(node); - } - - /// - /// Doesn't lock. - /// - private void UnsafeAdd(K key, V value, bool throwExceptionIfKeyExists) - { - if (_cache.TryGetValue(key, out var result)) - { - if (throwExceptionIfKeyExists) - { - throw new ArgumentException("Key already exists", nameof(key)); - } - else if (!result.Value.Equals(value)) - { - result.Value = value; - _cache[key] = result; - MoveNodeToTop(result.Node); - } - } - else - { - if (_cache.Count == _capacity) - { - UnsafeEvictLastNode(); - } - UnsafeAddNodeToTop(key, value); - } - } - - public V this[K key] - { - get - { - if (TryGetValue(key, out var value)) - { - return value; - } - else - { - throw new KeyNotFoundException(); - } - } - set - { - lock (_lockObject) - { - UnsafeAdd(key, value, false); - } - } - } - - public bool TryGetValue(K key, out V value) - { - lock (_lockObject) - { - return UnsafeTryGetValue(key, out value); - } - } - - /// - /// Doesn't lock. - /// - public bool UnsafeTryGetValue(K key, out V value) - { - if (_cache.TryGetValue(key, out var result)) - { - MoveNodeToTop(result.Node); - value = result.Value; - return true; - } - else - { - value = default(V); - return false; - } - } - - public V GetOrAdd(K key, V value) - { - lock (_lockObject) - { - if (UnsafeTryGetValue(key, out var result)) - { - return result; - } - else - { - UnsafeAdd(key, value, true); - return value; - } - } - } - - public V GetOrAdd(K key, Func creator) - { - lock (_lockObject) - { - if (UnsafeTryGetValue(key, out var result)) - { - return result; - } - else - { - var value = creator(); - UnsafeAdd(key, value, true); - return value; - } - } - } - - public V GetOrAdd(K key, T arg, Func creator) - { - lock (_lockObject) - { - if (UnsafeTryGetValue(key, out var result)) - { - return result; - } - else - { - var value = creator(arg); - UnsafeAdd(key, value, true); - return value; - } - } - } - } -} \ No newline at end of file diff --git a/src/Microsoft.Extensions.Logging.Abstractions/Internal/FormattedLogValues.cs b/src/Microsoft.Extensions.Logging.Abstractions/Internal/FormattedLogValues.cs index d5c9b929..1df1173b 100644 --- a/src/Microsoft.Extensions.Logging.Abstractions/Internal/FormattedLogValues.cs +++ b/src/Microsoft.Extensions.Logging.Abstractions/Internal/FormattedLogValues.cs @@ -3,6 +3,7 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; namespace Microsoft.Extensions.Logging.Internal @@ -14,16 +15,27 @@ namespace Microsoft.Extensions.Logging.Internal public class FormattedLogValues : IReadOnlyList> { private const string NullFormat = "[null]"; - private static ConcurrentLruCache _formatters = new ConcurrentLruCache(1024); + private static ConcurrentDictionary _formatters = new ConcurrentDictionary(); private readonly LogValuesFormatter _formatter; private readonly object[] _values; private readonly string _originalMessage; + private const int MaxCachedFormatters = 1024; public FormattedLogValues(string format, params object[] values) { if (values?.Length != 0 && format != null) { - _formatter = _formatters.GetOrAdd(format, format, f => new LogValuesFormatter(f)); + if (_formatters.Count >= MaxCachedFormatters) + { + if (!_formatters.TryGetValue(format, out _formatter)) + { + _formatter = new LogValuesFormatter(format); + } + } + else + { + _formatter = _formatters.GetOrAdd(format, f => new LogValuesFormatter(f)); + } } _originalMessage = format ?? NullFormat;