Skip to content

Remove the LightEpoch EntryAt accessor - #2033

Merged
Tiago Nápoli (tiagonapoli) merged 3 commits into
microsoft:mainfrom
tiagonapoli:tiagonapoli-lightepoch-entryat-cleanup
Aug 7, 2026
Merged

Tiago Nápoli (tiagonapoli) merged 3 commits into
microsoft:mainfrom
tiagonapoli:tiagonapoli-lightepoch-entryat-cleanup

Conversation

@tiagonapoli

@tiagonapoli Tiago Nápoli (tiagonapoli) commented Aug 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Follow-up cleanup to LightEpoch after #2015. Cosmetic only.

  • Remove EntryAt and inline every epoch table access as *(tableAligned + i). An AggressiveInlining accessor on the epoch hot path adds an inline frame in every caller and eats into the JIT's inlining budget for no benefit, so the raw dereference stays.

Copilot AI lite review requested due to automatic review settings August 6, 2026 17:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@tiagonapoli
Tiago Nápoli (tiagonapoli) force-pushed the tiagonapoli-lightepoch-entryat-cleanup branch from 82f9166 to 0de0373 Compare August 6, 2026 18:25
@tiagonapoli Tiago Nápoli (tiagonapoli) changed the title Standardize LightEpoch entry access on EntryAt and move the Entry struct to a partial file Move the LightEpoch Entry struct to a partial file and standardize the entry-table debug asserts Aug 6, 2026
@tiagonapoli
Tiago Nápoli (tiagonapoli) force-pushed the tiagonapoli-lightepoch-entryat-cleanup branch from 0de0373 to a916c07 Compare August 6, 2026 18:30
@tiagonapoli Tiago Nápoli (tiagonapoli) changed the title Move the LightEpoch Entry struct to a partial file and standardize the entry-table debug asserts Remove the LightEpoch EntryAt accessor and standardize the entry-table debug asserts Aug 6, 2026
@tiagonapoli
Tiago Nápoli (tiagonapoli) force-pushed the tiagonapoli-lightepoch-entryat-cleanup branch from a916c07 to 6a3b6f7 Compare August 6, 2026 18:34
…e debug asserts

Remove EntryAt and inline every epoch table access as `*(tableAligned + i)`, so
no helper sits on the epoch hot path.

Add DebugAssertEntryReserved / DebugAssertEntryNotReserved for the slot-reservation
checks. DebugAssertEpochAcquired now builds on the former, and Acquire's inline
`Debug.Assert(entry == kInvalidIndex)` goes through the latter.

Fold the ad-hoc `Debug.Assert(entry > 0, "Trying to refresh unacquired epoch")`
in ProtectAndDrain into DebugAssertEpochAcquired.

Co-authored-by: Copilot App <[email protected]>
@tiagonapoli
Tiago Nápoli (tiagonapoli) force-pushed the tiagonapoli-lightepoch-entryat-cleanup branch from 6a3b6f7 to 12e3607 Compare August 6, 2026 18:36
@tiagonapoli Tiago Nápoli (tiagonapoli) changed the title Remove the LightEpoch EntryAt accessor and standardize the entry-table debug asserts Remove the LightEpoch EntryAt accessor Aug 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

libs/storage/Tsavorite/cs/src/core/Epochs/LightEpoch.cs:618

  • This re-dereferences tableAligned + entry multiple times in a tight path. Consider caching a single ref to the slot (e.g., ref var slot = ref *(tableAligned + entry);) and then using slot.localCurrentEpoch / slot.threadId to reduce repeated address computation and improve readability without reintroducing a helper call frame.
            if ((*(tableAligned + entry)).localCurrentEpoch != 0)
                return false;

            var epoch = Volatile.Read(ref CurrentEpoch);
            if (Interlocked.CompareExchange(ref (*(tableAligned + entry)).localCurrentEpoch, epoch, 0) != 0)
                return false;

            // The slot is now exclusively ours, so threadId needs no interlocked write.
            (*(tableAligned + entry)).threadId = Metadata.threadId;

libs/storage/Tsavorite/cs/src/core/Epochs/LightEpoch.EntryTable.cs:18

  • Both assertions reuse the same primary message but provide different detail strings, which can lead to confusing output (and potentially multiple assertion dialogs) when index is invalid. Consider collapsing this into a single assertion with a clearer, specific detail for each failure mode (or early-return after the first failure) so debug failures are more actionable.
        private static void DebugAssertEntryReserved(int index, string message = "No epoch table entry is reserved for this thread")
        {
            Debug.Assert(index != kInvalidIndex, message, "No slot is reserved for this thread.");
            Debug.Assert(index > kInvalidIndex && index <= kTableSize, message, $"Slot {index} is out of range.");
        }

@tiagonapoli
Tiago Nápoli (tiagonapoli) merged commit 4ba5ebf into microsoft:main Aug 7, 2026
210 of 211 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants