xls: linear grid assembly; sqlite3: exact 64-bit integer decode - #17
Merged
Conversation
…rsor
GridFromCells used the old superlinear design PERF.md documents as the
original xlsb bottleneck: a Record.FromList row map keyed by row number
(record lookup cost grows with field count), a List.Accumulate row builder
(O(ncols^2) list append per row), and a Table.FromRecords -> Table.Group ->
Table.ToRecords round-trip.
Port xlsb's buffered index-range/run-cursor assembly: cells stay in one
List.Buffer, rows are kept as [A, B) index ranges (indexing cells{i} instead
of List.Range avoids the quadratic Skip walk), a single pointer-walk fills
each row in column order, and an unordered fallback sorts only when a
malformed file has out-of-order or repeated row headers.
Measured on the 24k-row ACE fixture: driverless eval 1871 ms -> 843 ms
(-55%), no functional-test changes.
Dispatch integer serial types in DecodeValue to hoisted big-endian BinaryFormat readers (SignedInteger16/32/64 for st 2/4/6, a byte plus manual sign for st 1), keeping the accumulate helper only for the 3- and 6-byte widths BinaryFormat cannot express, with precomputed sign constants. The 8-byte path via SignedInteger64 now decodes the full signed 64-bit range exactly, fixing the documented precision loss above 2^53; the types fixture (which deliberately stores 2^53+1 and the int64 extremes) is rebaselined to the true values. The precision notes in this reader's header, README and test doc are corrected, as are gpkg's and mbtiles' (they inherit this core). A/B on the 4M-row bulk fixture showed this dispatch is perf-neutral (that fixture stores its integers as 0/1 serial-type specials, so st 1-6 never runs); DecodeRecord is deliberately left as the original recursive walk, which the same A/B proved faster than a List.Generate cursor for narrow tables.
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.
Two independent reader improvements, each validated with a before/after A/B on the repo's own perf harness (
run-odbc-benchmark.ps1/run-script-benchmark.ps1, median of runs, warmup discarded, overhead-subtracted, with unchanged readers as thermal controls).1. xls — linear grid assembly (performance)
GridFromCellsused the old superlinear design PERF.md documents as the original xlsb bottleneck: aRecord.FromListrow map keyed by row number (record lookup cost grows with field count), aList.Accumulaterow builder (O(ncols²) append per row), and aTable.FromRecords→Table.Group→Table.ToRecordsround-trip.Replaced with xlsb's proven buffered run-cursor assembly: cells stay in one
List.Buffer, rows are kept as[A, B)index ranges (indexingcells{i}instead ofList.Rangeavoids the quadratic Skip walk), a single pointer-walk fills each row in column order, and anunorderedfallback sorts only when a malformed file has out-of-order/repeated row headers.Measured: driverless eval 1,871 ms → 843 ms (−55%) on the 24k-row ACE fixture; no functional-test changes.
2. sqlite3 — exact 64-bit integer decode (correctness, perf-neutral)
DecodeValuenow dispatches integer serial types to hoisted big-endianBinaryFormatreaders (SignedInteger16/32/64for st 2/4/6; a byte + manual sign for st 1), keeping the accumulate helper only for the 3- and 6-byte widthsBinaryFormatcan't express, with precomputed sign constants.The 8-byte path via
SignedInteger64decodes the full signed 64-bit range exactly, fixing the documented precision loss above 2^53. Thetypesfixture (which deliberately stores 2^53+1 and the int64 extremes) is rebaselined to the true values, and the precision notes in the reader header, README and test doc are corrected — plus gpkg's and mbtiles', which inherit this core.Measured: perf-neutral. The bulk fixture stores its integers as 0/1 serial-type specials (st 8/9), so st 1–6 never runs; the A/B confirmed no change (87.2 s ≈ 90 s before).
Reviewer note — what was deliberately not included
The audit's issue 2 also proposed rewriting
DecodeRecord(per-row header walk) as aList.Generatecursor. The A/B showed that regressed sqlite3 by +67% (86 s → 144 s on the 4M-row fixture, rippling +17% to gpkg/mbtiles):List.Generate's per-step overhead is paid on every row, and the "O(ncols²)" it targets is negligible at 4 columns.DecodeRecordis therefore left as the original recursive walk. Only the perf-neutral, correctness-improvingDecodeValuehalf is kept.