docs: give each explanation one home, compress the core comments - #13
Merged
Conversation
Four explanations were repeated between the core's comments and docs/, and each repetition was a place they could drift apart. They now live in one place each, and the comments state the invariant rather than re-arguing it. Moved out of the code, in full, not summarised away: the measured cost of not filling a shadow on a miss -> docs/design.md what demotion costs SIEVE and S3-FIFO specifically -> docs/policies.md MinEpochRequests counts sampled, not real, requests -> docs/configuration.md MinShadowCapacity raises the effective rate -> docs/configuration.md The last two were in the godoc and nowhere else, so a reader of the configuration guide could set either one wrong: at a 0.05 sample rate a MinEpochRequests of 100 is reached after roughly 2000 real requests, not 100. docs/policies.md also gains the three rules an arm has to honour, which until now were recorded only in a working file outside the repository: never return a zero value with true, keep Keys() and Values() aligned, and read size 0 as empty rather than unlimited. Each has been broken by a real implementation -- the first is why policies.NewTTL is written over a plain LRU rather than wrapping expirable. Comment density across the six core files, which the stage set out to bring under 25%: cache.go 109/361 = 30% -> 100/352 = 28% epoch.go 65/207 = 31% -> 64/206 = 31% shadow.go 114/215 = 53% -> 56/157 = 35% sampling.go 53/129 = 41% -> 41/117 = 35% migration.go 47/137 = 34% -> 43/133 = 32% settings.go 91/196 = 46% -> 61/166 = 36% total 479/1245 = 38% -> 365/1131 = 32% It stops at 32%, not 25%. What remains is invariants, lock preconditions and godoc on exported API; reaching the number would mean deleting documentation whose absence lets someone break correctness by editing nearby -- the switchLocked ordering rule, the three roles during a gradual window, why Peek and not Contains. The target and the rule that no removed comment may describe a load-bearing invariant conflict on these files, and the invariant wins. The README needed no work: it is 56 lines against a target of 90.
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.
Four explanations were repeated between the core's comments and
docs/, andeach repetition was a place they could drift apart. They now live in one place
each, and the comments state the invariant instead of re-arguing it.
Explanation → canonical anchor
docs/design.md(Idea, step 3)shadow.gofanOutReadLockeddocs/policies.mdshadow.godemoteLockedMinEpochRequestscounts sampled, not real, requestsdocs/configuration.mdsettings.gogodoc onlyMinShadowCapacityraises the effective ratedocs/configuration.mdsettings.gogodoc onlydocs/design.md+sampling.go(the arithmetic)settings.go,shadow.godocs/design.mdThe two marked only were in godoc and nowhere else, so a reader of the
configuration guide could set either wrong. At a 0.05 sample rate a
MinEpochRequestsof 100 is reached after roughly 2000 real requests, not 100.Invariants recovered from outside the repository
docs/policies.mdgains the three rules an arm has to honour, each broken by areal implementation: never return a zero value with
true, keepKeys()andValues()aligned, read size 0 as empty rather than unlimited (and do notstart a goroutine with no way to stop it). The first is why
policies.NewTTLis written over a plain LRU rather than wrapping
expirable.Comment density
It stops at 32%, not the 25% the stage asked for. What remains is
invariants, lock preconditions, and godoc on exported API. Reaching the number
would mean deleting documentation whose absence lets someone break correctness
by editing nearby — the
switchLockedordering rule, the three roles during agradual window, why
Peekand notContains. The target and the rule that noremoved comment may describe a load-bearing invariant conflict on these files,
and the invariant wins. Flagging rather than quietly missing it.
The README needed no work: 56 lines against a target of 90.
Verification
Class G — please read before merging:
README.md(unchanged), and thediffs of
shadow.goandepoch.go. Those two are where a deleted commentcould have taken an invariant with it.