From c3c2cc2e61411fc1c56ca962684348ccf5563ce4 Mon Sep 17 00:00:00 2001 From: Eirik George Tsarpalis Date: Thu, 29 Aug 2024 21:02:19 +0000 Subject: [PATCH 1/4] Fix performance issue when deserializing large payloads in JsonObject extension data. Bug fix to address performance issues when deserializing large payloads in `JsonObject` extension data. Mitigates performance issues by optimizing the deserialization process for large `JsonObject` extension data properties. - Added `LargeJsonObjectExtensionDataSerializationState` class to temporarily store properties in a dictionary for efficient updates. - Modified `JsonObjectConverter` to use the new state class for large objects. - Updated `ObjectDefaultConverter` and `ObjectWithParameterizedConstructorConverter` to complete deserialization using the new state class. - Added tests in `JsonObjectTests` to validate performance improvements with large payloads. --- ...icrosoft.Extensions.DependencyModel.csproj | 2 + .../src/System.Text.Json.csproj | 5 +- .../Converters/Node/JsonObjectConverter.cs | 14 +++-- ...onObjectExtensionDataSerializationState.cs | 53 +++++++++++++++++++ .../Object/ObjectDefaultConverter.cs | 3 ++ ...ctWithParameterizedConstructorConverter.cs | 3 ++ .../Text/Json/Serialization/ReadStackFrame.cs | 2 + .../JsonNode/JsonObjectTests.cs | 29 ++++++++++ 8 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/LargeJsonObjectExtensionDataSerializationState.cs diff --git a/src/libraries/Microsoft.Extensions.DependencyModel/src/Microsoft.Extensions.DependencyModel.csproj b/src/libraries/Microsoft.Extensions.DependencyModel/src/Microsoft.Extensions.DependencyModel.csproj index 81b60b7135205..6fca8a4c7aec9 100644 --- a/src/libraries/Microsoft.Extensions.DependencyModel/src/Microsoft.Extensions.DependencyModel.csproj +++ b/src/libraries/Microsoft.Extensions.DependencyModel/src/Microsoft.Extensions.DependencyModel.csproj @@ -2,6 +2,8 @@ netstandard2.0;net461 true + true + 1 Abstractions for reading `.deps` files. Commonly Used Types: diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index f478aabb427fd..c3bb2b0b586be 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -9,8 +9,8 @@ enable true true - false - 9 + true + 10 Provides high-performance and low-allocating types that serialize objects to JavaScript Object Notation (JSON) text and deserialize JSON text to objects, with UTF-8 support built-in. Also provides types to read and write JSON text encoded as UTF-8, and to create an in-memory document object model (DOM), that is read-only, for random access of the JSON elements within a structured view of the data. Commonly Used Types: @@ -106,6 +106,7 @@ System.Text.Json.Utf8JsonReader + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs index b25f544d3c911..56d3625d8334f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs @@ -26,10 +26,18 @@ internal override void ReadElementAndSetProperty( Debug.Assert(obj is JsonObject); JsonObject jObject = (JsonObject)obj; - Debug.Assert(value == null || value is JsonNode); - JsonNode? jNodeValue = (JsonNode?)value; + if (jObject.Count < LargeJsonObjectExtensionDataSerializationState.LargeObjectThreshold) + { + jObject[propertyName] = value; + } + else + { + LargeJsonObjectExtensionDataSerializationState deserializationState = + state.Current.LargeJsonObjectExtensionDataSerializationState ??= new(jObject); - jObject[propertyName] = jNodeValue; + Debug.Assert(ReferenceEquals(deserializationState.Destination, jObject)); + deserializationState.AddProperty(propertyName, value); + } } public override void Write(Utf8JsonWriter writer, JsonObject value, JsonSerializerOptions options) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/LargeJsonObjectExtensionDataSerializationState.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/LargeJsonObjectExtensionDataSerializationState.cs new file mode 100644 index 0000000000000..a9f0e7abd4a88 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/LargeJsonObjectExtensionDataSerializationState.cs @@ -0,0 +1,53 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Text.Json.Nodes; + +namespace System.Text.Json.Serialization.Converters +{ + /// + /// Implements a mitigation for deserializing large JsonObject extension data properties. + /// Extension data properties use replace semantics when duplicate keys are encountered, + /// which is an O(n) operation for JsonObject resulting in O(n^2) total deserialization time. + /// This class mitigates the performance issue by storing the deserialized properties in a + /// temporary dictionary (which has O(1) updates) and copies them to the destination object + /// at the end of deserialization. + /// + internal sealed class LargeJsonObjectExtensionDataSerializationState + { + public const int LargeObjectThreshold = 25; + private readonly Dictionary _tempDictionary; + public JsonObject Destination { get; } + + public LargeJsonObjectExtensionDataSerializationState(JsonObject destination) + { + StringComparer comparer = destination.Options?.PropertyNameCaseInsensitive ?? false + ? StringComparer.OrdinalIgnoreCase + : StringComparer.Ordinal; + + Destination = destination; + _tempDictionary = new(comparer); + } + + /// + /// Stores a deserialized property to the temporary dictionary, using replace semantics. + /// + public void AddProperty(string key, JsonNode? value) + { + _tempDictionary[key] = value; + } + + /// + /// Copies the properties from the temporary dictionary to the destination JsonObject. + /// + public void Complete() + { + // Because we're only appending values to _tempDictionary, this should preserve JSON ordering. + foreach (KeyValuePair kvp in _tempDictionary) + { + Destination[kvp.Key] = kvp.Value; + } + } + } +} diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs index c12debeb61856..a8a0837a7ccd8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs @@ -243,6 +243,9 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert, jsonTypeInfo.UpdateSortedPropertyCache(ref state.Current); } + // Complete any JsonObject extension data deserializations. + state.Current.LargeJsonObjectExtensionDataSerializationState?.Complete(); + return true; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs index f6420c6bf4de9..5e98546e04b17 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs @@ -165,6 +165,9 @@ internal sealed override bool OnTryRead(ref Utf8JsonReader reader, Type typeToCo state.Current.JsonTypeInfo.UpdateSortedParameterCache(ref state.Current); } + // Complete any JsonObject extension data deserializations. + state.Current.LargeJsonObjectExtensionDataSerializationState?.Complete(); + return true; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs index 137305f1d5b43..ae78b6af30003 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Converters; using System.Text.Json.Serialization.Metadata; namespace System.Text.Json @@ -34,6 +35,7 @@ internal struct ReadStackFrame public JsonTypeInfo JsonTypeInfo; public StackFrameObjectState ObjectState; // State tracking the current object. + public LargeJsonObjectExtensionDataSerializationState? LargeJsonObjectExtensionDataSerializationState; // Validate EndObject token on array with preserve semantics. public bool ValidateEndTokenOnArray; diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs index 1f152606a7e7f..34a749f9c019d 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Text.Json.Serialization; using System.Text.Json.Serialization.Tests; using Xunit; @@ -920,5 +921,33 @@ public static void ChangeCollectionWhileEnumeratingFails(JsonObject jObject, int }); Assert.Equal(1, index); } + + [Theory] + [InlineData(10_000)] + [InlineData(50_000)] + [InlineData(100_000)] + public static void JsonObject_ExtensionData_ManyDuplicatePayloads(int size) + { + // Generate the payload + StringBuilder builder = new StringBuilder(); + builder.Append("{"); + for (int i = 0; i < size; i++) + { + builder.Append($"\"{i}\": 0,"); + builder.Append($"\"{i}\": 0,"); + } + builder.Length--; // strip trailing comma + builder.Append("}"); + + string jsonPayload = builder.ToString(); + ClassWithObjectExtensionData result = JsonSerializer.Deserialize(jsonPayload); + Assert.Equal(size, result.ExtensionData.Count); + } + + class ClassWithObjectExtensionData + { + [JsonExtensionData] + public JsonObject ExtensionData { get; set; } + } } } From 40d60a122dfd637e1b8a7cccbbbc363d508fb0d7 Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Mon, 9 Sep 2024 16:54:58 +0000 Subject: [PATCH 2/4] ZipArchive: Improve performance or removing extra fields This improves the performance of the `ZipArchive` by optimizing the removal of extra fields. - `src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs`: Replaced multiple iterations and list operations with a single `RemoveAll` call to remove specific `ZipGenericExtraField` entries. --- .../src/System/IO/Compression/ZipBlocks.cs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs index 058a8617dea61..ac3673eef0a7a 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipBlocks.cs @@ -237,14 +237,12 @@ public static Zip64ExtraField GetAndRemoveZip64Block(List zip64Field._localHeaderOffset = null; zip64Field._startDiskNumber = null; - List markedForDelete = new List(); bool zip64FieldFound = false; - foreach (ZipGenericExtraField ef in extraFields) + extraFields.RemoveAll(ef => { if (ef.Tag == TagConstant) { - markedForDelete.Add(ef); if (!zip64FieldFound) { if (TryGetZip64BlockFromGenericExtraField(ef, readUncompressedSize, readCompressedSize, @@ -253,24 +251,18 @@ public static Zip64ExtraField GetAndRemoveZip64Block(List zip64FieldFound = true; } } + return true; } - } - foreach (ZipGenericExtraField ef in markedForDelete) - extraFields.Remove(ef); + return false; + }); return zip64Field; } public static void RemoveZip64Blocks(List extraFields) { - List markedForDelete = new List(); - foreach (ZipGenericExtraField field in extraFields) - if (field.Tag == TagConstant) - markedForDelete.Add(field); - - foreach (ZipGenericExtraField field in markedForDelete) - extraFields.Remove(field); + extraFields.RemoveAll(field => field.Tag == TagConstant); } public void WriteBlock(Stream stream) From 741b04589cdd3514d032cbfd041fb7178bbd15c9 Mon Sep 17 00:00:00 2001 From: "Eric St. John" Date: Mon, 9 Sep 2024 16:58:54 +0000 Subject: [PATCH 3/4] Packaging: Port fix for GetParts from .NETFramework This addresses a bug in the `GetParts` method by porting a fix from .NETFramework, improving part URI handling and collision detection.. `src/libraries/System.IO.Packaging/src/System/IO/Packaging/Package.cs`: Added sorting of parts array, updated dictionary structure for part URIs, and implemented collision detection and handling. `src/libraries/System.IO.Packaging/src/System.IO.Packaging.csproj`: Enabled package generation on build and updated servicing version. --- .../src/System.IO.Packaging.csproj | 6 +- .../src/System/IO/Packaging/Package.cs | 62 ++++++++++++++++--- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.IO.Packaging/src/System.IO.Packaging.csproj b/src/libraries/System.IO.Packaging/src/System.IO.Packaging.csproj index 3444e2515ce83..0936526a24ddd 100644 --- a/src/libraries/System.IO.Packaging/src/System.IO.Packaging.csproj +++ b/src/libraries/System.IO.Packaging/src/System.IO.Packaging.csproj @@ -7,8 +7,8 @@ $(NoWarn);CA1847 true - false - 0 + true + 1 Provides classes that support storage of multiple data objects in a single container. @@ -59,4 +59,4 @@ - \ No newline at end of file + diff --git a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/Package.cs b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/Package.cs index 09c9b5c307534..caf13d4bd0fd0 100644 --- a/src/libraries/System.IO.Packaging/src/System/IO/Packaging/Package.cs +++ b/src/libraries/System.IO.Packaging/src/System/IO/Packaging/Package.cs @@ -403,32 +403,63 @@ public PackagePartCollection GetParts() PackUriHelper.ValidatedPartUri partUri; + var uriComparer = Comparer.Default; + + //Sorting the parts array which takes O(n log n) time. + Array.Sort(parts, Comparer.Create((partA, partB) => uriComparer.Compare((PackUriHelper.ValidatedPartUri)partA.Uri, (PackUriHelper.ValidatedPartUri)partB.Uri))); + //We need this dictionary to detect any collisions that might be present in the //list of parts that was given to us from the underlying physical layer, as more than one //partnames can be mapped to the same normalized part. //Note: We cannot use the _partList member variable, as that gets updated incrementally and so its //not possible to find the collisions using that list. //PackUriHelper.ValidatedPartUri implements the IComparable interface. - Dictionary seenPartUris = new Dictionary(parts.Length); + Dictionary> partDictionary = new Dictionary>(parts.Length); + List partIndex = new List(parts.Length); for (int i = 0; i < parts.Length; i++) { partUri = (PackUriHelper.ValidatedPartUri)parts[i].Uri; - if (seenPartUris.ContainsKey(partUri)) + string normalizedPartName = partUri.NormalizedPartUriString; + + if (partDictionary.ContainsKey(normalizedPartName)) + { throw new FileFormatException(SR.BadPackageFormat); + } else { - // Add the part to the list of URIs that we have already seen - seenPartUris.Add(partUri, parts[i]); + //since we will arive to this line of code after the parts are already sorted + string? precedingPartName = null; + + if (partIndex.Count > 0) + { + precedingPartName = (partIndex[partIndex.Count - 1]); + } + + // Add the part to the dictionary + partDictionary.Add(normalizedPartName, new KeyValuePair(partUri, parts[i])); - if (!_partList.ContainsKey(partUri)) + if (precedingPartName != null + && normalizedPartName.StartsWith(precedingPartName, StringComparison.Ordinal) + && normalizedPartName.Length > precedingPartName.Length + && normalizedPartName[precedingPartName.Length] == PackUriHelper.ForwardSlashChar) { - // Add the part to the _partList if there is no prefix collision - AddIfNoPrefixCollisionDetected(partUri, parts[i]); + //Removing the invalid entry from the _partList. + partDictionary.Remove(normalizedPartName); + + throw new InvalidOperationException(SR.PartNamePrefixExists); } + + //adding entry to partIndex to keep track of last element being added. + //since parts are already sorted, last element in partIndex list will point to preceeding element to the current. + partIndex.Add(partUri.NormalizedPartUriString); } } + + //copying parts from partdictionary to partlist + CopyPartDicitonaryToPartList(partDictionary, partIndex); + _partCollection = new PackagePartCollection(_partList); } return _partCollection; @@ -1186,6 +1217,23 @@ private PackageRelationshipCollection GetRelationshipsHelper(string? filterStrin return new PackageRelationshipCollection(_relationships, filterString); } + private void CopyPartDicitonaryToPartList(Dictionary> partDictionary, List partIndex) + { + //Clearing _partList before copying in new data. Reassigning the variable, assuming the previous object to be garbage collected. + //ideally addition to sortedlist takes O(n) but since we have sorted data and also we defined the size, it will take O(log n) per addition + //total time complexity for this function will be O(n log n) + _partList = new SortedList(partDictionary.Count); + + //Since partIndex is created from a sorted parts array we are sure that partIndex + //will have items in same order + foreach (var id in partIndex) + { + //retrieving object from partDictionary hashtable + var keyValue = partDictionary[id]; + _partList.Add(keyValue.Key, keyValue.Value); + } + } + #endregion Private Methods #region Private Members From fd1aa2c473164cc81476dc1933333bb4af26698f Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Tue, 10 Sep 2024 01:39:53 +0000 Subject: [PATCH 4/4] Microsoft.Extensions.Caching.Memory: use Marvin for keys on down-level TFMs Note that Microsoft.Extensions.Caching.Memory is deployed OOB via NuGet, and multi-targets including support for netfx and ns2.0 Marvin is the string hashing used in modern .NET (https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Marvin.cs); this change extends this behaviour to `string` keys when used with `MemoryCache` - noting that in many scenarios we *expect* the key to be a `string` (even though other types are allowed) To do this we: - ~~add a custom key equality comparer for use with `ConcurrentDictionary` (we do this on all TFMs, replacing the need for the dynamic lookup via `EqualityComparer.Default`)~~ - split the internal dictionary into 2 - one for `string`, one for everything else - in down-level TFMs only, provide a custom `string` key comparer with `MarvinHash` enabled (local snapshot since not available) This gives us Marvin everywhere, and Marvin+rehash on netcore TFMs (rehash requires `TKey` === `string`) --- .../src/MemoryCache.cs | 144 +++++++++++++++--- ...Microsoft.Extensions.Caching.Memory.csproj | 8 +- .../src/System/Marvin.cs | 38 ++++- 3 files changed, 166 insertions(+), 24 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index d9fa37e8cf834..391fb17c3c541 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -2,9 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections; using System.Collections.Concurrent; using System.Collections.Generic; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Internal; @@ -23,7 +25,8 @@ public class MemoryCache : IMemoryCache internal readonly ILogger _logger; private readonly MemoryCacheOptions _options; - private readonly ConcurrentDictionary _entries; + private readonly ConcurrentDictionary _stringKeyEntries; + private readonly ConcurrentDictionary _nonStringKeyEntries; private long _cacheSize; private bool _disposed; @@ -56,7 +59,8 @@ public MemoryCache(IOptions optionsAccessor, ILoggerFactory _options = optionsAccessor.Value; _logger = loggerFactory.CreateLogger(); - _entries = new ConcurrentDictionary(); + _stringKeyEntries = new ConcurrentDictionary(StringKeyComparer.Instance); + _nonStringKeyEntries = new ConcurrentDictionary(); if (_options.Clock == null) { @@ -74,12 +78,14 @@ public MemoryCache(IOptions optionsAccessor, ILoggerFactory /// /// Gets the count of the current entries for diagnostic purposes. /// - public int Count => _entries.Count; + public int Count => _stringKeyEntries.Count + _nonStringKeyEntries.Count; // internal for testing internal long Size { get => Interlocked.Read(ref _cacheSize); } - private ICollection> EntriesCollection => _entries; + private ICollection> StringKeyEntriesCollection => _stringKeyEntries; + + private ICollection> NonStringKeyEntriesCollection => _nonStringKeyEntries; /// public ICacheEntry CreateEntry(object key) @@ -129,7 +135,16 @@ internal void SetEntry(CacheEntry entry) // Initialize the last access timestamp at the time the entry is added entry.LastAccessed = utcNow; - if (_entries.TryGetValue(entry.Key, out CacheEntry priorEntry)) + CacheEntry priorEntry = null; + string s = entry.Key as string; + if (s != null) + { + if (_stringKeyEntries.TryGetValue(s, out priorEntry)) + { + priorEntry.SetExpired(EvictionReason.Replaced); + } + } + else if (_nonStringKeyEntries.TryGetValue(entry.Key, out priorEntry)) { priorEntry.SetExpired(EvictionReason.Replaced); } @@ -143,12 +158,26 @@ internal void SetEntry(CacheEntry entry) if (priorEntry == null) { // Try to add the new entry if no previous entries exist. - entryAdded = _entries.TryAdd(entry.Key, entry); + if (s != null) + { + entryAdded = _stringKeyEntries.TryAdd(s, entry); + } + else + { + entryAdded = _nonStringKeyEntries.TryAdd(entry.Key, entry); + } } else { // Try to update with the new entry if a previous entries exist. - entryAdded = _entries.TryUpdate(entry.Key, entry, priorEntry); + if (s != null) + { + entryAdded = _stringKeyEntries.TryUpdate(s, entry, priorEntry); + } + else + { + entryAdded = _nonStringKeyEntries.TryUpdate(entry.Key, entry, priorEntry); + } if (entryAdded) { @@ -163,7 +192,14 @@ internal void SetEntry(CacheEntry entry) // The update will fail if the previous entry was removed after retrival. // Adding the new entry will succeed only if no entry has been added since. // This guarantees removing an old entry does not prevent adding a new entry. - entryAdded = _entries.TryAdd(entry.Key, entry); + if (s != null) + { + entryAdded = _stringKeyEntries.TryAdd(s, entry); + } + else + { + entryAdded = _nonStringKeyEntries.TryAdd(entry.Key, entry); + } } } @@ -223,7 +259,18 @@ public bool TryGetValue(object key, out object result) DateTimeOffset utcNow = _options.Clock.UtcNow; - if (_entries.TryGetValue(key, out CacheEntry entry)) + bool found; + CacheEntry entry; + if (key is string s) + { + found = _stringKeyEntries.TryGetValue(s, out entry); + } + else + { + found = _nonStringKeyEntries.TryGetValue(key, out entry); + } + + if (found) { // Check if expired due to expiration tokens, timers, etc. and if so, remove it. // Allow a stale Replaced value to be returned due to concurrent calls to SetExpired during SetEntry. @@ -262,7 +309,18 @@ public void Remove(object key) ValidateCacheKey(key); CheckDisposed(); - if (_entries.TryRemove(key, out CacheEntry entry)) + bool removed; + CacheEntry entry; + if (key is string s) + { + removed = _stringKeyEntries.TryRemove(s, out entry); + } + else + { + removed = _nonStringKeyEntries.TryRemove(key, out entry); + } + + if (removed) { if (_options.SizeLimit.HasValue) { @@ -278,7 +336,17 @@ public void Remove(object key) private void RemoveEntry(CacheEntry entry) { - if (EntriesCollection.Remove(new KeyValuePair(entry.Key, entry))) + bool removed; + if (entry.Key is string s) + { + removed = StringKeyEntriesCollection.Remove(new KeyValuePair(s, entry)); + } + else + { + removed = NonStringKeyEntriesCollection.Remove(new KeyValuePair(entry.Key, entry)); + } + + if (removed) { if (_options.SizeLimit.HasValue) { @@ -317,10 +385,8 @@ private static void ScanForExpiredItems(MemoryCache cache) { DateTimeOffset now = cache._lastExpirationScan = cache._options.Clock.UtcNow; - foreach (KeyValuePair item in cache._entries) + foreach (CacheEntry entry in cache.GetCacheEntries()) { - CacheEntry entry = item.Value; - if (entry.CheckExpired(now)) { cache.RemoveEntry(entry); @@ -388,10 +454,26 @@ private static void OvercapacityCompaction(MemoryCache cache) /// ?. Larger objects - estimated by object graph size, inaccurate. public void Compact(double percentage) { - int removalCountTarget = (int)(_entries.Count * percentage); + int removalCountTarget = (int)(Count * percentage); Compact(removalCountTarget, _ => 1); } + private IEnumerable GetCacheEntries() + { + // note this mimics the outgoing code in that we don't just access + // .Values, which has additional overheads; this is only used for rare + // calls - compaction, clear, etc - so the additional overhead of a + // generated enumerator is not alarming + foreach (KeyValuePair item in _stringKeyEntries) + { + yield return item.Value; + } + foreach (KeyValuePair item in _nonStringKeyEntries) + { + yield return item.Value; + } + } + private void Compact(long removalSizeTarget, Func computeEntrySize) { var entriesToRemove = new List(); @@ -403,9 +485,8 @@ private void Compact(long removalSizeTarget, Func computeEntry // Sort items by expired & priority status DateTimeOffset now = _options.Clock.UtcNow; - foreach (KeyValuePair item in _entries) + foreach (CacheEntry entry in GetCacheEntries()) { - CacheEntry entry = item.Value; if (entry.CheckExpired(now)) { entriesToRemove.Add(entry); @@ -526,5 +607,34 @@ private static void ValidateCacheKey(object key) static void Throw() => throw new ArgumentNullException(nameof(key)); } + +#if NETCOREAPP + // on .NET Core, the inbuilt comparer has Marvin built in; no need to intercept + private static class StringKeyComparer + { + internal static IEqualityComparer Instance => EqualityComparer.Default; + } +#else + // otherwise, we need a custom comparer that manually implements Marvin + private sealed class StringKeyComparer : IEqualityComparer, IEqualityComparer + { + private StringKeyComparer() { } + + internal static readonly IEqualityComparer Instance = new StringKeyComparer(); + + // special-case string keys and use Marvin hashing + public int GetHashCode(string? s) => s is null ? 0 + : Marvin.ComputeHash32(MemoryMarshal.AsBytes(s.AsSpan()), Marvin.DefaultSeed); + + public bool Equals(string? x, string? y) + => string.Equals(x, y); + + bool IEqualityComparer.Equals(object x, object y) + => object.Equals(x, y); + + int IEqualityComparer.GetHashCode(object obj) + => obj is string s ? Marvin.ComputeHash32(MemoryMarshal.AsBytes(s.AsSpan()), Marvin.DefaultSeed) : 0; + } +#endif } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/Microsoft.Extensions.Caching.Memory.csproj b/src/libraries/Microsoft.Extensions.Caching.Memory/src/Microsoft.Extensions.Caching.Memory.csproj index a65eda71abed6..d7a09ae1d5a9b 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/Microsoft.Extensions.Caching.Memory.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/Microsoft.Extensions.Caching.Memory.csproj @@ -4,7 +4,9 @@ netstandard2.0;net461 true In-memory cache implementation of Microsoft.Extensions.Caching.Memory.IMemoryCache. - 1 + true + 2 + true @@ -15,4 +17,8 @@ + + + + diff --git a/src/libraries/System.Private.CoreLib/src/System/Marvin.cs b/src/libraries/System.Private.CoreLib/src/System/Marvin.cs index 322b7888c34fb..6601e89f90bd6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Marvin.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Marvin.cs @@ -2,10 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Numerics; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; + +#if SYSTEM_PRIVATE_CORELIB using Internal.Runtime.CompilerServices; +using static System.Numerics.BitOperations; +#else +using System.Security.Cryptography; +#endif namespace System { @@ -205,7 +210,7 @@ public static int ComputeHash32(ref byte data, uint count, uint p0, uint p1) else { partialResult |= (uint)Unsafe.ReadUnaligned(ref data); - partialResult = BitOperations.RotateLeft(partialResult, 16); + partialResult = RotateLeft(partialResult, 16); } } @@ -222,16 +227,16 @@ private static void Block(ref uint rp0, ref uint rp1) uint p1 = rp1; p1 ^= p0; - p0 = BitOperations.RotateLeft(p0, 20); + p0 = RotateLeft(p0, 20); p0 += p1; - p1 = BitOperations.RotateLeft(p1, 9); + p1 = RotateLeft(p1, 9); p1 ^= p0; - p0 = BitOperations.RotateLeft(p0, 27); + p0 = RotateLeft(p0, 27); p0 += p1; - p1 = BitOperations.RotateLeft(p1, 19); + p1 = RotateLeft(p1, 19); rp0 = p0; rp1 = p1; @@ -242,8 +247,29 @@ private static void Block(ref uint rp0, ref uint rp1) private static unsafe ulong GenerateSeed() { ulong seed; +#if SYSTEM_PRIVATE_CORELIB Interop.GetRandomBytes((byte*)&seed, sizeof(ulong)); +#else + byte[] seedBytes = new byte[sizeof(ulong)]; + using (RandomNumberGenerator rng = RandomNumberGenerator.Create()) + { + rng.GetBytes(seedBytes); + fixed (byte* b = seedBytes) + { + seed = *(ulong*)b; + } + } +#endif return seed; } + +#if !SYSTEM_PRIVATE_CORELIB + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static uint RotateLeft(uint value, int shift) + { + // This is expected to be optimized into a single rol (or ror with negated shift value) instruction + return (value << shift) | (value >> (32 - shift)); + } +#endif } }