Conversation
Three structural changes, no behaviour change:
- The 100-byte file header is one hoisted BinaryFormat.Record over a
100-byte slice instead of four magic-offset reads (UInt2(16), Byte(18),
Byte(20), UInt4(56)). A length guard keeps the "Not a SQLite 3 database
file" error for files too short to hold a header, which Binary.Range
would otherwise fail on differently.
- The cell pointer array is contiguous, so it is read with one slice and
one BinaryFormat.List rather than ncells separate slice+parse calls.
- DecodeValue dispatches serial types 0-9 through a hoisted list of
readers instead of a ten-branch if-chain, so text and blob no longer
fall through every integer comparison first. Reserved types 10/11 now
error directly rather than reaching the same error via SerialSize.
Both header and cell pointer array are sequential structures, which is
where combinators belong (rule 5 in context/PERF.md); the random-access
b-tree traversal keeps its Binary.Range slicing.
DecodeRecord was left alone on purpose. Its `acc & {x}` appends are
O(ncols^2) per row, but converting that walk to a List.Generate cursor
regressed measurably in 2026-07 and again now: List.Generate costs two
closure calls and two state-record allocations per column on every row,
and returns a lazy list that must then be buffered before the INTEGER
PRIMARY KEY fixup can index it. A comment there records why, so the next
reader of that code does not have to rediscover it.
Measured neutral on tests/perf sqlite3-int-decode (5 passes, rotated
order, medians within noise). Full PQTest suite green.
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.
Three structural changes, no behaviour change:
DecodeRecord was left alone on purpose. Its
acc & {x}appends are O(ncols^2) per row, but converting that walk to a List.Generate cursor regressed measurably in 2026-07 and again now: List.Generate costs two closure calls and two state-record allocations per column on every row, and returns a lazy list that must then be buffered before the INTEGER PRIMARY KEY fixup can index it. A comment there records why, so the next reader of that code does not have to rediscover it.Measured neutral on tests/perf sqlite3-int-decode (5 passes, rotated order, medians within noise). Full PQTest suite green.