Open
Conversation
485a6c2 to
738ebf1
Compare
Replace the fixed-size InstanceIndexBuffer [ThreadStatic] struct (previously limited to 16 concurrent instances) with a dynamically-growing GC-pinned int[] accessed via a [ThreadStatic] int* pointer. This removes the hard cap on concurrent LightEpoch instances entirely. Design: - Per-thread entries array allocated as pinned int[] on first Resume() per thread, starting at capacity 16 and doubling as needed - Hot-path access is uniform pointer arithmetic: *(entriesPtr + instanceId), no branches - Instance IDs allocated via atomic counter with ConcurrentQueue recycling of disposed IDs to keep per-thread arrays compact - Double-dispose guarded via Interlocked.Exchange - ThisInstanceProtected() includes null + bounds checks for pre-Resume() calls Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
738ebf1 to
9b772b3
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the hard cap on concurrent LightEpoch instances by replacing the fixed-size per-thread instance-index buffer with a lazily allocated, dynamically growing GC-pinned int[] accessed via a [ThreadStatic] int* pointer. It updates both the Tsavorite core and client copies of LightEpoch to keep behavior aligned.
Changes:
- Replaces the fixed-size
[ThreadStatic]instance index buffer with a per-thread pinnedint[]+int*pointer that grows by doubling. - Switches instance ID allocation to an atomic counter with a
ConcurrentQueue<int>recycle pool and tracks active instances via an atomic counter. - Adds double-dispose guarding and updates
ThisInstanceProtected()to include null + bounds checks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| libs/storage/Tsavorite/cs/src/core/Epochs/LightEpoch.cs | Implements dynamic per-thread pinned instance-entry map and new global instance ID allocation/recycling in Tsavorite core. |
| libs/client/LightEpoch.cs | Mirrors the same dynamic per-thread pinned instance-entry map and instance ID allocation/recycling in the client copy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void Release() | ||
| { | ||
| ref var entry = ref Metadata.Entries.GetRef(instanceId); | ||
| Debug.Assert(Metadata.entriesPtr != null, "Release called before Resume on this thread"); |
Comment on lines
+669
to
+672
| var requiredCapacity = instanceId + 1; | ||
| var newCapacity = Math.Max(kInitialEntriesCapacity, Metadata.entriesCapacity); | ||
| while (newCapacity < requiredCapacity) | ||
| newCapacity *= 2; |
Comment on lines
+187
to
192
| Interlocked.Increment(ref activeInstanceCount); | ||
| if (freeInstanceIds.TryDequeue(out var recycledId)) | ||
| return recycledId; | ||
| return Interlocked.Increment(ref nextInstanceId) - 1; | ||
| } | ||
|
|
Comment on lines
+278
to
280
| Debug.Assert(Metadata.entriesPtr != null, "ProtectAndDrain called before Resume on this thread"); | ||
| ref var entry = ref *(Metadata.entriesPtr + instanceId); | ||
|
|
| void Release() | ||
| { | ||
| ref var entry = ref Metadata.Entries.GetRef(instanceId); | ||
| Debug.Assert(Metadata.entriesPtr != null, "Release called before Resume on this thread"); |
Comment on lines
+670
to
+673
| var requiredCapacity = instanceId + 1; | ||
| var newCapacity = Math.Max(kInitialEntriesCapacity, Metadata.entriesCapacity); | ||
| while (newCapacity < requiredCapacity) | ||
| newCapacity *= 2; |
Comment on lines
185
to
+190
| int SelectInstance() | ||
| { | ||
| for (var i = 0; i < InstanceIndexBuffer.MaxInstances; i++) | ||
| { | ||
| ref var entry = ref InstanceTracker.GetRef(i); | ||
| // Try to claim this instance ID (indicated as 1 in the entry) | ||
| if (kInvalidIndex == Interlocked.CompareExchange(ref entry, 1, kInvalidIndex)) | ||
| return i; | ||
| } | ||
| throw new InvalidOperationException($"Exceeded maximum number of active LightEpoch instances {ActiveInstanceCount()} {InstanceIndexBuffer.MaxInstances}"); | ||
| Interlocked.Increment(ref activeInstanceCount); | ||
| if (freeInstanceIds.TryDequeue(out var recycledId)) | ||
| return recycledId; | ||
| return Interlocked.Increment(ref nextInstanceId) - 1; |
Comment on lines
+278
to
281
| Debug.Assert(Metadata.entriesPtr != null, "ProtectAndDrain called before Resume on this thread"); | ||
| ref var entry = ref *(Metadata.entriesPtr + instanceId); | ||
|
|
||
| Debug.Assert(entry > 0, "Trying to refresh unacquired epoch"); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the fixed-size InstanceIndexBuffer [ThreadStatic] struct (previously limited to 16 concurrent instances) with a dynamically-growing GC-pinned int[] accessed via a [ThreadStatic] int* pointer. This removes the hard cap on concurrent LightEpoch instances entirely.
Design: