refactor(array): propagate allocators through builders - #9670
Conversation
9f76879 to
cb223eb
Compare
cb223eb to
dd4f747
Compare
dd4f747 to
fd018db
Compare
fd018db to
737f66c
Compare
737f66c to
08a209a
Compare
56d0be5 to
4af7bf3
Compare
01b019a to
de08d68
Compare
de08d68 to
b773833
Compare
b773833 to
4b92e40
Compare
62670ea to
f494e5a
Compare
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | chunked_bool_canonical_into[(1000, 10)] |
27.1 µs | 30.8 µs | -12.25% |
| ⚡ | WallTime | arrow_checked_add_u32_neon[16384] |
20.5 µs | 12.8 µs | +59.85% |
| ⚡ | Simulation | decompress[u64, (4000, 1024)] |
85.6 µs | 71.2 µs | +20.3% |
| ⚡ | Simulation | allocate_drop_arrow[0] |
456.9 ns | 402.7 ns | +13.45% |
| ⚡ | Simulation | compact_sliced[(1024, 90)] |
1.5 µs | 1.4 µs | +12.03% |
| ⚡ | Simulation | compact_sliced[(2048, 90)] |
1.6 µs | 1.4 µs | +11.53% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ngates/buffer-allocator-builders (69a9f2e) with develop (792cef7)
Footnotes
-
206 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. ↩
0ce40e7 to
9076b5d
Compare
9076b5d to
66cd9f7
Compare
66cd9f7 to
4e3ecb4
Compare
4e3ecb4 to
4fe4b22
Compare
d5d1c39 to
fd3a728
Compare
fd3a728 to
cfcc653
Compare
cfcc653 to
f497774
Compare
f497774 to
a8608ae
Compare
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
a8608ae to
f627f7c
Compare
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
|
No breaks please, can you add back old api and mark it deprecated. Users don't want the churn just because you need a new feature. |
|
I dunno… most callers will have an ExecutionCtx in scope and not pass it. In fact, shouldn't ArrayBuilders hold ExecutionCtx anyway so that you can extend_from_array? |
|
I am purely arguing from the churn POV, it would be ideal if we can avoid forcing people to update their builder usage when they upgrade vortex. I think we can remove it in 1-2 releases |
Summary
Require every array builder caller to choose its buffer allocator explicitly.
Changes
_inconstructor pairs with one allocator-taking API.ctx.allocator()in execution paths; use the static allocator only where no execution context exists.