Repository navigation
MementoFilter reads each table straight into its segment, holding it once (#327) - #339
Merged
Merged
Conversation
…(red) (#327) Two tests, as #313 has for InfiniFilter. TestAReadHoldsEachTableOnce (TestMementoFilter) reads a filter of 2-bit fingerprints built for 2,048 keys and given 48,000, whose chain holds tables of 98,344 and 12,328 bytes, and holds what the read allocates, counted in a no-GC region, and what it is charged, to its payload and one copy of its tables plus 4 KiB: in the ordinary envelope, and in the large form, where the payload is not held, to the tables alone. Each is held from below too. Both tables are past the slack, which a premise checks. Red on 5752769 (fix/329-read-path-leftovers, on 05a5669), at the first check: the read allocated 333,024 bytes, 111,676 more than its payload and its tables That is the second copy of the tables (110,672) and 1,004 besides. The test prints every figure first, and on 5752769 they are: payload 110,676, tables 110,672 ordinary read 333,024, 111,676 past payload + tables ordinary charge 332,312, 110,964 past payload + tables large-form read 222,424, 111,752 past the tables large-form charge 221,712, 111,040 past the tables TestAMementoFiltersTableIsReadFromTheLargeFormInPiecesStraightIntoIts- Segment (TestLargeEnvelopeReading) holds the budget sample's table, found as the bytes its words are written as, to being read in pieces of the shipped 16 KiB through the stream's span overload, one after another, with nothing among them, and in pieces of eight words with the piece lowered, as the Infini test does. Red on 5752769: the table's 73,744 bytes at 58, with the piece at 16,384, were read in 9,218 reads, the first of 8 bytes through the span overload, and not in 5 pieces through the span overload which is the reader's loop of ReadUInt64, a word asked of the stream at a time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KK3MspqAcczxT45xCfLgyx
) The reader read a table's words a word at a time (ReadUInt64) into an array of their own, then MementoSegment.Restore copied that array into the table it built, so a read held, and was charged for, every table twice, as InfiniFilter's did before #313. The segment is now built first and its words are read straight into its table with PayloadReader.ReadWords(Span<ulong>), the overload #329 made of Infini's and the cuckoo filter's helpers, in pieces through a stack buffer of BlockPieceFor (16 KiB). Restore no longer takes the words. The all-zero check for a 64-bit slot's table, which the read loop kept as it went, now asks the table (ContainsAnyExcept(0)). The checks before the allocation stand where they were: CheckSegment, which holds the words to the shape and to one array (#326), and RequireHolds, which holds the payload to the words (#322), both run before the table is charged or allocated. SegmentCost is one table and the segment, not two tables, and the price uses it as the reader does, so price and charge still come to one sum (TestReadBudget holds them equal for every sample). The segment is charged as two small objects, 128 bytes: a MementoSegment and its KeepsakeCodec come to 120 bytes beyond the table, measured in a no-GC region at tables of 2^2, 2^8 and 2^12 slots, where one small object, 64, was charged beside the second table. Charged one table and one small object, every read would have been 56 bytes a table under what it allocates. TestASegmentIsChargedWhatRestoringItAllocates holds the charge to at least what Restore allocates and less than a small object past it, as #329's test does for Infini. Measured on the red test's filter (2-bit fingerprints, 48,000 keys, tables of 98,344 and 12,328 bytes), in a no-GC region: 5752769 this ordinary read 333,024 222,352 ordinary charge 332,312 221,768 large-form read 222,424 111,752 large-form charge 221,712 111,168 Past the payload in the ordinary envelope, and in all in the large form, what a read allocates and is charged is halved: 222,348 to 111,676 and 221,636 to 111,092, 222,424 to 111,752 and 221,712 to 111,168. The pieces test reads the sample's 73,744-byte table in 5 reads through the span overload (1,153 with the piece lowered), where it took 9,218. Documents moved by it, through the capacity checks: - README: the default budget now reads a range of 65,536 up to a table of 2^27 slots, 100,663,296 items (was 50,331,648); a range of 256 stays at 2^27. - CAPACITY.md: the note said a range of 65,536 reads back only 2^26, with no claim behind it. It now says a read holds each table twice, in the payload and in the table, and that the default budget reads back 2^27 slots at both ranges, each held by a claim: the charge lines' slopes are 16 bytes a word at both ranges (were 24), and the fitted lines reach 2^27 and not 2^28. - FORMAT.md: MementoFilter's row of read charges is merged with InfiniFilter's, which it now reads as. TestAMementoTableNoArrayHoldsIsRefusedByNameBeforeAnythingIsPricedOrAll- ocated (#326) held the widest table an array holds to a price of at least its words twice over, the old double charge, and failed: 2^31 slots of 63 bits priced 16,911,433,896 bytes. It now asks for the words once and less than twice over, so that a price that kept the second table fails it too. Mutation, scripts/mutate.sh, MUTATION_JOBS=2, at this commit's tree (a984cf6, before this table was added to its message), whole suite unless a test is named in brackets: read into a copy, copy it in (defect) killed (4) read a word at a time, no copy killed (1) segment: two tables (5752769) killed (5) segment: one small object killed (1) segment: three small objects killed (1) zero-table check inverted killed (5) reader: hold check dropped (#322) killed (3) price: a second table killed (6) reader: segment not charged killed (2) [holds once] read into a copy killed (1) [holds once] two tables killed (1) [pieces] read a word at a time killed (1) [segment] one small object killed (1) [segment] three small objects killed (1) "Read into a copy" is the defect under the new charge: the words read into an array of their own and copied into the segment's table. "A word at a time" is the old loop of ReadUInt64, into the table, so it allocates what the fix does; only the pieces test sees it. "Two tables" puts back the old SegmentCost, by which price and charge agree, so only what is measured sees it. The segment test's mutants show its two bounds bite, and one small object is the charge that would have undercharged by 56 bytes a table, which nothing else kills. The test could not be red before the fix, which changes Restore's signature and makes SegmentCost visible; these are its failures. "Hold check dropped" removes #322's RequireHolds from the reader, so a payload cut inside a table has the table allocated before its words are found missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KK3MspqAcczxT45xCfLgyx
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KK3MspqAcczxT45xCfLgyx
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.
Fixes #327.
Stacked on the #329 PR (parts 1–3), for its shared
ReadWordsoverload. GitHub retargets this PR to master once that one merges.MementoFilter's reader no longer holds each table twice. It now reads the words straight into the segment's storage, as #313 does forInfiniFilter. That is the coordinator's plan on #327, approved by Matt.The change
MementoSegment.Restoreno longer takes the words. The reader reads them straight intosegment.Datathrough Read-path leftovers: ReadBytes' empty-run charge, a segment undercharge, duplicate ReadWords, an unused allowance mechanism #329's overload.CheckSegment, which includes MementoFilter: a table no single array holds is refused by name, before it is priced or allocated #326's one-array check, and Readers and their prices refuse a payload that claims more than it holds, before charging or allocating for it #322'sRequireHoldsstill run before the table is charged or allocated.SegmentCostis now one table plus two small objects. A segment and its codec measured 120 bytes, so charging one table and one small object would undercharge by 56 bytes a table.CapacityClaims:Measured in
NoGCRegion, on two tables of 110,672 bytes:What's held beyond the payload is halved, and for the large form so is the whole read.
Tests
TestAReadHoldsEachTableOnce: red with "allocated 333,024 bytes, 111,676 more".TestASegmentIsChargedWhatRestoringItAllocates: this one couldn't exist before the fix, becauseRestore's signature changed. Mutation shows it bites: it alone kills the "one small object" mutant.Mutation: 14 of 14 killed. The table is in
291a56c's message.Verification
-warnaserror: exit 0.🤖 Generated with Claude Code
https://claude.ai/code/session_01KK3MspqAcczxT45xCfLgyx