Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch caching file comparer to a threadstatic storage to prevent locks #75297

Merged
merged 5 commits into from
Sep 30, 2024
Merged

Switch caching file comparer to a threadstatic storage to prevent locks #75297

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4770657
Switch to a threadlocal storage to prevent locks
CyrusNajmabadi Sep 30, 2024
caf4583
Simplify
CyrusNajmabadi Sep 30, 2024
727f6c6
Simpler
CyrusNajmabadi Sep 30, 2024
646c980
use ref
CyrusNajmabadi Sep 30, 2024
d972d95
stoub
CyrusNajmabadi Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 12 additions & 55 deletions src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CachingFilePathComparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,12 @@ private sealed class CachingFilePathComparer : IEqualityComparer<string>
public static readonly CachingFilePathComparer Instance = new();

/// <summary>
/// Lock to protect the the last string and hash code we computed. `enableThreadOwnerTracking: false` as we
/// don't need that tracking, and it substantially speeds up the spin lock (removing 0.7% cpu from solution load
/// scenario).
/// ThreadStatic so that gets its own copy it can safely read/write from, removing the need for expensive
/// contentious locks. The purpose of this type is to allow lookup of the same key across N dictionaries
/// efficiently from the same thread. So this accomplishes that purpose.
/// </summary>
private SpinLock _lock = new(enableThreadOwnerTracking: false);
private string? _lastString;
private int _lastHashCode;
[ThreadStatic]
private static (string? lastString, int lastHashCode) s_data;

private CachingFilePathComparer()
{
Expand All @@ -51,61 +50,19 @@ public bool Equals(string? x, string? y)

public int GetHashCode([DisallowNull] string obj)
{
if (TryGetCachedHashCode(obj, out var hashCode))
return hashCode;
// SToub thinks this may be faster on NetFx as it will help the runtime with reading/writing from a single location.
ref var data = ref s_data;
if (ReferenceEquals(data.lastString, obj))
return data.lastHashCode;

// Hashing a different string than last time. Compute the hash and cache the value.

// Specialized impl of OrdinalIgnoreCase.GetHashCode that is faster for the common case of an all-ASCII
// string. Falls back to normal OrdinalIgnoreCase.GetHashCode for the uncommon case.
hashCode = GetNonRandomizedHashCodeOrdinalIgnoreCase(obj);
var hash = GetNonRandomizedHashCodeOrdinalIgnoreCase(obj);

var lockTaken = false;
try
{
_lock.Enter(ref lockTaken);
_lastString = obj;
_lastHashCode = hashCode;
}
finally
{
if (lockTaken)
_lock.Exit();
}

return hashCode;
}

private bool TryGetCachedHashCode(string obj, out int hashCode)
{
var lastString = _lastString;

// Quickly check if this is definitely *not* trying to hash the same string that this comparer was just used
// to hash. If that's the case, we can avoid taking the lock and just return false immediately. For the
// case when a lot of distinct strings are being hashed (say, when a dictionary is being populated), this
// means we only spin-wait once.
if (!ReferenceEquals(lastString, obj))
{
hashCode = default;
return false;
}

// Otherwise, take the lock, and now copy both the string and hash out.
var lockTaken = false;
try
{
_lock.Enter(ref lockTaken);
lastString = _lastString;
hashCode = _lastHashCode;
}
finally
{
if (lockTaken)
_lock.Exit();
}

// Check again, as another thread may have written into this field between the first check and taking the lock.
return ReferenceEquals(lastString, obj);
data = (obj, hash);
return hash;
}

// From https://github.com/dotnet/runtime/blob/5aa9687e110faa19d1165ba680e52585a822464d/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs#L921
Expand Down
Loading