Repository navigation
Speed up scalar construction, stats reads and aggregate partial merging - #10326
Conversation
Merging this PR will improve performance by 31.08%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | sum_v2 |
91 µs | 27.1 µs | ×3.4 |
| ⚡ | Simulation | density_sweep_dense_runs[0.001] |
47.7 µs | 30.3 µs | +57.3% |
| ⚡ | Simulation | filter_powerlaw_by_mostly_true[250000] |
150 µs | 99.5 µs | +50.79% |
| ⚡ | Simulation | is_valid_per_element[1024] |
405.9 µs | 280.5 µs | +44.71% |
| ⚡ | Simulation | is_valid_per_element[256] |
104.5 µs | 72.3 µs | +44.48% |
| ⚡ | Simulation | repeated[primitive] |
238.8 µs | 176 µs | +35.62% |
| ⚡ | Simulation | execute_scalar_struct_wide |
444.9 µs | 333.2 µs | +33.52% |
| ⚡ | Simulation | once[primitive] |
272.9 µs | 211.1 µs | +29.27% |
| ⚡ | Simulation | repeated[struct] |
701.3 µs | 557.7 µs | +25.75% |
| ⚡ | Simulation | once[dict] |
497.6 µs | 401.5 µs | +23.93% |
| ⚡ | Simulation | repeated[dict] |
508.2 µs | 410.7 µs | +23.72% |
| ⚡ | Simulation | once[struct] |
785.5 µs | 643.7 µs | +22.03% |
| ⚡ | Simulation | scalar_access[(1024, false, false)] |
727.7 µs | 599 µs | +21.48% |
| ⚡ | Simulation | execute_scalar_struct_simple |
99.1 µs | 83.9 µs | +18.06% |
| ⚡ | Simulation | scalar_access[(1024, true, false)] |
787.8 µs | 670.5 µs | +17.49% |
| ⚡ | Simulation | search_index_mixed_out_of_range_chunked |
373.7 µs | 330.5 µs | +13.09% |
| ⚡ | Simulation | search_index_below_min_chunked |
375.4 µs | 332.8 µs | +12.78% |
| ⚡ | Simulation | search_index_full_range_random_chunked |
405.6 µs | 363.1 µs | +11.68% |
| ⚡ | Simulation | probe_scalar_fn_valid_once[vortex.binary(bool?, len=100000)] |
317.3 µs | 288 µs | +10.18% |
| ⚡ | Simulation | probe_scalar_fn_once[vortex.binary(bool?, len=100000)] |
262.1 µs | 238.2 µs | +10.02% |
| ... | ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing rk/scalar-construction-perf (5087c16) with develop (58a3c2f)
Footnotes
-
518 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. ↩
e33f04f to
0e63743
Compare
Validation, not copying, dominated scalar construction. `Scalar::try_new` now inlines a fast compatibility check and moves error construction into a cold path. Constructors whose value is valid by construction skip it. Stats reads and merges build fewer scalars, SumV2 parses partials from the tuple values, `combine_partials` and `partial_from_scalar` take `&Scalar`, and accumulators made from one bound aggregate function share their dtypes. Also fixes `merge_is_constant`, which always overwrote its result with `exact(false)`. Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XUNmEACSgmThaNJMLtH61K
A merge costs about 300ns, so 1024 merges per iteration would take the CodSpeed simulation well over 1ms. Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XUNmEACSgmThaNJMLtH61K Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XUNmEACSgmThaNJMLtH61K
Signed-off-by: Robert Kruszewski <github@robertk.io> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XUNmEACSgmThaNJMLtH61K
3cf195b to
5087c16
Compare
Summary
Make stats validation cheap, skips it where values are valid by construction, and removes
Scalarconstruction and copies from stats reads and aggregate partial merging.Changes
Scalar::try_newandScalar::validaterun an inline fast check. Error messages are built in a cold path.new_unchecked. VarBinView reads no longer check UTF-8 again.Scalar.AggregateFnRefshare their resolved dtypes, so their partial dtypes compare by pointer.merge_is_constantalways overwrote its result withexact(false).scalar_constructionbench.try_new, primitiveexecute_scalar, primitive / utf8 / struct8statistics().get(Min)StatsSet::merge_orderedcombine_partialsAPI Changes
AggregateFnVTable::partial_from_scalar,DynAccumulator::combine_partialsandMinMaxResult::from_scalartake&Scalar.StructFields::field_dtypes()returns&DTypewithout cloning.IsConstantcan now beexact(true). When the result cannot be known, the stat is cleared.