Repository navigation
Let a slab that is out of nodes borrow a look before it takes one - #2
Merged
Merged
Conversation
A look at the readers' slots goes through every slot there is, and a slab can make do with the look that another slab of the cache has taken. That is what keeps the cost of a write from growing with the number of CPUs. But only writes that evict got to borrow. A write that replaces a value takes its new node before it retires the old one, and on a full slab it ran out of spare nodes just before the count of waiting nodes passed the slab's patience, which is where borrowing was tried. So it always looked for itself: once for every 32 writes in a shard of 256 entries, where evicting writes look once in 1,350. reclaim now says what it is for. A slab that needs a node borrows first and looks only if that got it none. On a full cache of 16,384 entries, a replacing write from one goroutine took 38 ns with the slots of 14 CPUs, 52 ns with those of 64 and 104 ns with those of 256; it takes 35 to 37 ns with any of them now. A node whose reader has left can wait one round longer than before if the borrowed look still names it. That stays within the patience that the packages promise. Co-Authored-By: Claude <noreply@anthropic.com>
The test gave a goroutine 20 ms to find every spare node pinned, then let go of one and expected the slab to have counted a wait. On a slow macOS runner under the race detector the goroutine had not got that far, found a node at its first try, and the test failed. It now waits until the writer has looked at the readers' slots twice, which it can only have done by coming away empty-handed once. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Replacing writes in
sieveands3fifonever shared a scan of the readers' slots, although evicting writes did. This makes them share.The defect. A slab reuses the nodes of entries that have left only after a look at every reader slot, and it can borrow the look that another slab of the same cache has taken.
retiretried to borrow once more nodes were waiting than the slab has patience for. ButReplacecallsallocbeforeretire, and on a full slaballocruns out of spare nodes when exactlypatiencenodes are waiting, one short of that. There it looked for itself without trying to borrow.The change.
reclaimtakes a reason:forRoom(too many nodes waiting: borrow, then look if still too many),forOne(a node is needed: borrow, look only if that released none),forSure(Reclaim,Released: always look).Effect. With 16 slabs of 256 entries, 200,000 replacing writes took 6,243 looks before and take 528 now; evicting writes take 525 either way. A replacing write from one goroutine on a full cache of 16,384 entries, by the number of CPUs the cache is sized for:
The published benchmark results are left as they are: with 8 CPUs or fewer the difference is about 1%, within the ranges shown there.
Checks. New tests:
TestSharedLooks(fails without the fix),TestAllocBorrowsAroundPin,TestAllocAndTooManyPins. The mutation suite kills all 99 mutants, including five new ones for this logic. An independent review found no safety or liveness defect; its two tests are the last two above. One behavior changes within what the docs promise: a node whose reader has left can wait one round longer if the borrowed look still names it.🤖 Generated with Claude Code