Canonicalize chunked nested types through the builder [builders-child-stack] - #8967
Canonicalize chunked nested types through the builder [builders-child-stack]#8967robert3005 wants to merge 5 commits into
Conversation
Merging this PR will degrade performance by 18.45%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | canonicalize[1024, 32] |
56.2 µs | 75.1 µs | -25.18% |
| ❌ | Simulation | canonicalize[256, 32] |
56.3 µs | 74.5 µs | -24.44% |
| ❌ | Simulation | canonicalize[16, 32] |
58.3 µs | 74.9 µs | -22.12% |
| ❌ | Simulation | canonicalize[1024, 8] |
43.8 µs | 54.4 µs | -19.52% |
| ❌ | Simulation | canonicalize[256, 8] |
43.8 µs | 53.1 µs | -17.64% |
| ❌ | Simulation | canonicalize[16, 8] |
47 µs | 55.1 µs | -14.68% |
| ❌ | WallTime | words_gather_scalar[65536] |
8.2 µs | 9.4 µs | -12.39% |
| ❌ | Simulation | canonicalize[1024, 2] |
41.5 µs | 46.3 µs | -10.29% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/chunked-canonical-via-builder-9ze0t6 (0746486) with develop (adfdd77)2
Footnotes
-
42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
develop(b052a06) during the generation of this report, so adfdd77 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
afed52f to
79f350a
Compare
1145029 to
8f39b06
Compare
8f39b06 to
825ab2e
Compare
825ab2e to
ce2eb1d
Compare
ce2eb1d to
cefbc8e
Compare
cefbc8e to
0d4eb9a
Compare
0d4eb9a to
1e8346a
Compare
1e8346a to
0431fa7
Compare
a9f7d31 to
9c8574c
Compare
9c8574c to
0b003e4
Compare
876aa4e to
a66422f
Compare
a66422f to
dac04cb
Compare
dac04cb to
3f93504
Compare
697f31a to
5f173bf
Compare
5f173bf to
15bd75e
Compare
`pack_struct_chunks`, `swizzle_fixed_size_list_chunks` and `swizzle_list_chunks` existed because `append_to_builder` used to decode a builder's children: without them, canonicalizing a `ChunkedArray` would concatenate every chunk's children instead of reusing them. Their doc comments describe what the builders now do on their own, so all three are dead weight - the generic builder path produces the same swizzled array, with each chunk's children kept as chunks of the combined child. Only `Variant` still needs a hand-written pack, because there is no variant builder. This drops the one canonicalization path that allocated its buffers through the session allocator: `swizzle_list_chunks` allocated its offsets and sizes with `ctx.allocator()`, whereas `builder_with_capacity_in` still ignores the allocator it is handed, so chunked primitives, structs and FSLs already went around it. `list_canonicalize_uses_memory_session_allocator` guarded that one path and goes with it; restoring the property means teaching the builders to allocate through a `HostAllocatorRef`, not keeping a bespoke list swizzle alive. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <robert@spiraldb.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
15bd75e to
f01c3f9
Compare
I, Robert Kruszewski <github@robertk.io>, hereby add my Signed-off-by to this commit: 5c8401d Signed-off-by: Robert Kruszewski <github@robertk.io>
Always use builders when canonicalizing chunked arrays