Add copy-on-write write-path counters#51
Open
MesTTo wants to merge 2 commits into
Open
Conversation
The counters module rotted while the feature was off: it imports ZipperPriv, which no longer exists (get_focus moved to ZipperInfallibleSubtries, already in the zipper glob), and its doc example references an undefined map and leaves print_traversal's free V parameter uninferred. With the import dropped and the example given a real map and a turbofish, cargo build/test/doc all pass with --features counters.
Extends the counters feature with the write-side split it was missing: every structural write funnels through TrieNodeODRc::make_unique, which either finds the node unshared or pays the copy-on-write clone. Two process-wide relaxed atomics record that split (make_unique_calls / cow_clones), exposed as a CowCounters snapshot with a reset, so a workload's write amplification under aliasing is directly measurable. The hook is two lines in make_unique, compiled out entirely without the feature (the default build is unchanged). An integration test pins the semantics in its own process: unshared writes record zero clones, writing an aliased trie records at least one and never more clones than calls, the aliased handle stays untouched, and dropping the alias stops the cloning.
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
1 participant
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.
Summary
A follow-up to #46, which you correctly flagged as dead plumbing (the
record sites and readers had zero callers). This time the writer and reader
are both wired to real code:
TrieNodeODRc::make_unique— the one choke pointevery structural write in the library goes through — record whether the
call found the node already unique or had to pay the copy-on-write clone.
Compiled out entirely without the
countersfeature; the default build isbyte-for-byte unchanged.
cow_counters()/reset_cow_counters()return aCowCounters { make_unique_calls, cow_clones }snapshot, exercised by anintegration test in its own process: writing an unshared trie records zero
clones, writing while another handle aliases it records at least one clone
(and never more clones than calls), the aliased handle is unaffected, and
writes after the alias is dropped stop cloning.
Also includes a standalone fix: the
countersfeature didn't compile onmaster(an import of azipper_priv::ZipperPrivthat no longer exists —get_focusmoved to the publicZipperInfallibleSubtries— plus a docexample referencing an undefined
mapand leaving a generic parameteruninferred). Fixed as its own commit before adding the new counters.
Test plan
cargo test --release --features counters(660/0 lib, 9/0 doc)cargo test --release --features counters --test cow_counters(1/1)cargo test --releasewith the feature off (660/0, unchanged)