diff --git a/src/StringTools/WeakStringCache.Concurrent.cs b/src/StringTools/WeakStringCache.Concurrent.cs index 42c06414dae..8a0bd78fca3 100644 --- a/src/StringTools/WeakStringCache.Concurrent.cs +++ b/src/StringTools/WeakStringCache.Concurrent.cs @@ -13,13 +13,14 @@ namespace Microsoft.NET.StringTools /// internal sealed partial class WeakStringCache : IDisposable { - private readonly ConcurrentDictionary _stringsByHashCode; + private const int WeakHandleMinimumLength = 500; + private readonly ConcurrentDictionary _stringsByHashCode; private readonly ConcurrentDictionary _weakHandlesByHashCode; private int _count; public WeakStringCache() { - _stringsByHashCode = new ConcurrentDictionary(Environment.ProcessorCount, _initialCapacity); + _stringsByHashCode = new ConcurrentDictionary(Environment.ProcessorCount, _initialCapacity); _weakHandlesByHashCode = new ConcurrentDictionary(Environment.ProcessorCount, _initialCapacity); } @@ -35,90 +36,105 @@ public string GetOrCreateEntry(ref InternableString internable, out bool cacheHi { int hashCode = internable.GetHashCode(); - StringWeakHandle? handle; - string? result; - // We use separate caches for string references and weak handles, as only the latter requires taking a lock // on the handle. bool usingWeakHandle = internable.Length > WeakHandleMinimumLength; - ConcurrentDictionary stringsByHashCode = usingWeakHandle ? _weakHandlesByHashCode : _stringsByHashCode; - if (stringsByHashCode.TryGetValue(hashCode, out handle)) - { - // Lock a weak handle while we're dereferencing it to prevent a race with the Scavenge - // method running on another thread and freeing the handle from underneath us. - if (usingWeakHandle) - { - Monitor.Enter(handle); - } + return usingWeakHandle ? + GetStringFromWeakHandle(ref internable, out cacheHit, hashCode) : + GetString(ref internable, out cacheHit, hashCode); - try + string GetString(ref InternableString internable, out bool cacheHit, int hashCode) + { + ConcurrentDictionary stringsByHashCode = _stringsByHashCode; + if (stringsByHashCode.TryGetValue(hashCode, out string? result)) { - result = handle.GetString(ref internable); - if (result != null) + if (internable.Equals(result)) { cacheHit = true; return result; } + } - // We have the handle but it's not referencing the right string - create the right string and store it in the handle. - result = internable.ExpensiveConvertToString(); - handle.SetString(result); + cacheHit = false; + result = internable.ExpensiveConvertToString(); + stringsByHashCode[hashCode] = result; // Update the cache with the new string. - cacheHit = false; - return result; - } - finally + return result; + } + + string GetStringFromWeakHandle(ref InternableString internable, out bool cacheHit, int hashCode) + { + string? result; + ConcurrentDictionary weakHandlesByHashCode = _weakHandlesByHashCode; + if (weakHandlesByHashCode.TryGetValue(hashCode, out StringWeakHandle? handle)) { - if (usingWeakHandle) + // Lock a weak handle while we're dereferencing it to prevent a race with the Scavenge + // method running on another thread and freeing the handle from underneath us. + Monitor.Enter(handle); + + try + { + result = handle.GetString(ref internable); + if (result != null) + { + cacheHit = true; + return result; + } + + // We have the handle but it's not referencing the right string - create the right string and store it in the handle. + result = internable.ExpensiveConvertToString(); + handle.SetString(result); + + cacheHit = false; + return result; + } + finally { Monitor.Exit(handle); } } - } - // We don't have the handle in the cache - create the right string, store it in the handle, and add the handle to the cache. - result = internable.ExpensiveConvertToString(); + // We don't have the handle in the cache - create the right string, store it in the handle, and add the handle to the cache. + result = internable.ExpensiveConvertToString(); - handle = new StringWeakHandle(); - handle.SetString(result); - if (stringsByHashCode.TryAdd(hashCode, handle)) - { - // Scavenge only clears out weak handles. - if (usingWeakHandle) + handle = new StringWeakHandle(); + handle.SetString(result); + if (weakHandlesByHashCode.TryAdd(hashCode, handle)) { + // Scavenge only clears out weak handles. Interlocked.Increment(ref _count); } - } - // Remove unused handles if our heuristic indicates that it would be productive. - int scavengeThreshold = _scavengeThreshold; - if (usingWeakHandle && _count >= scavengeThreshold) - { - // Before we start scavenging set _scavengeThreshold to a high value to effectively lock other threads from - // running Scavenge at the same time. - if (Interlocked.CompareExchange(ref _scavengeThreshold, int.MaxValue, scavengeThreshold) == scavengeThreshold) + // Remove unused handles if our heuristic indicates that it would be productive. + int scavengeThreshold = _scavengeThreshold; + if (_count >= scavengeThreshold) { - try + // Before we start scavenging set _scavengeThreshold to a high value to effectively lock other threads from + // running Scavenge at the same time. + if (Interlocked.CompareExchange(ref _scavengeThreshold, int.MaxValue, scavengeThreshold) == scavengeThreshold) { - // Get rid of unused handles. - Scavenge(); - } - finally - { - // And do this again when the number of handles reaches double the current after-scavenge number. - _scavengeThreshold = _weakHandlesByHashCode.Count * 2; - - // This count is not exact, since there can be some Interlocked.Increment(ref _count); - // calls happening due to this not being behind a lock. - // e.g. code checks if (_stringsByHashCode.TryAdd(hashCode, handle)), we set the _count here and the code increments - // however since this is just a threshold to scavenge, it should be fine to be off by few even if that happens. - _count = _weakHandlesByHashCode.Count; + try + { + // Get rid of unused handles. + Scavenge(); + } + finally + { + // And do this again when the number of handles reaches double the current after-scavenge number. + _scavengeThreshold = _weakHandlesByHashCode.Count * 2; + + // This count is not exact, since there can be some Interlocked.Increment(ref _count); + // calls happening due to this not being behind a lock. + // e.g. code checks if (_stringsByHashCode.TryAdd(hashCode, handle)), we set the _count here and the code increments + // however since this is just a threshold to scavenge, it should be fine to be off by few even if that happens. + _count = _weakHandlesByHashCode.Count; + } } } - } - cacheHit = false; - return result; + cacheHit = false; + return result; + } } /// diff --git a/src/StringTools/WeakStringCache.cs b/src/StringTools/WeakStringCache.cs index 891dc267e33..1fb0816b8ff 100644 --- a/src/StringTools/WeakStringCache.cs +++ b/src/StringTools/WeakStringCache.cs @@ -16,8 +16,6 @@ namespace Microsoft.NET.StringTools /// internal sealed partial class WeakStringCache : IDisposable { - private const int WeakHandleMinimumLength = 500; - /// /// Debug stats returned by GetDebugInfo(). /// @@ -35,17 +33,12 @@ private class StringWeakHandle /// /// Weak GC handle to the last string of the given hashcode we've seen. /// - public GCHandle WeakHandle; - - /// - /// Reference used for smaller strings retained by the cache. - /// - private string? referencedString; + private GCHandle weakHandle; /// /// Returns true if the string referenced by the handle is still alive. /// - public bool IsUsed => referencedString is not null || WeakHandle.Target != null; + public bool IsUsed => weakHandle.IsAllocated && weakHandle.Target != null; /// /// Returns the string referenced by this handle if it is equal to the given internable. @@ -54,18 +47,12 @@ private class StringWeakHandle /// The string matching the internable or null if the handle is referencing a collected string or the string is different. public string? GetString(ref InternableString internable) { - string? localReferenceString = referencedString; - if (localReferenceString is not null && internable.Equals(localReferenceString)) - { - return localReferenceString; - } - - if (!WeakHandle.IsAllocated) + if (!weakHandle.IsAllocated) { return null; } - if (WeakHandle.Target is not string str) + if (weakHandle.Target is not string str) { return null; } @@ -84,27 +71,13 @@ private class StringWeakHandle /// The string to set. public void SetString(string str) { - if (str.Length > WeakHandleMinimumLength) + if (weakHandle.IsAllocated) { - if (WeakHandle.IsAllocated) - { - WeakHandle.Target = str; - } - else - { - WeakHandle = GCHandle.Alloc(str, GCHandleType.Weak); - } - - referencedString = null; + weakHandle.Target = str; } else { - if (WeakHandle.IsAllocated) - { - WeakHandle.Target = null; - } - - referencedString = str; + weakHandle = GCHandle.Alloc(str, GCHandleType.Weak); } } @@ -113,9 +86,9 @@ public void SetString(string str) /// public void Free() { - if (WeakHandle.IsAllocated) + if (weakHandle.IsAllocated) { - WeakHandle.Free(); + weakHandle.Free(); } } }