Remove len parameter from ScalarFnArray::try_new#8378
Conversation
ScalarFnArray::try_newlen parameter from ScalarFnArray::try_new
Merging this PR will improve performance by 36.9%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | chunked_bool_canonical_into[(1000, 10)] |
35.5 µs | 20.5 µs | +73.18% |
| ⚡ | Simulation | chunked_varbinview_canonical_into[(1000, 10)] |
198.1 µs | 161.5 µs | +22.63% |
| ⚡ | Simulation | chunked_varbinview_into_canonical[(1000, 10)] |
213.5 µs | 176.8 µs | +20.81% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ct/remove-len-scalar-fn-array (ea3ad14) with develop (8475910)
Footnotes
-
10 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. ↩
Pull request was converted to draft
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
9409852 to
49ece36
Compare
|
@claude fix the merge conflicts |
|
Claude finished @connortsui20's task in 3m 54s —— View job Merge conflict analysis
What's conflicting
Of the 25 files in this PR, 15 no longer exist on
The remaining 10 files carry the actual API change (removing
Why I couldn't push the fixThis review environment denies the git operations required to build the merge commit:
So I can't create or push the merge commit from this job. To enable it, add Recommended resolution (≈1 min locally)git switch ct/remove-len-scalar-fn-array
git fetch origin develop
git merge origin/develop
# All conflicts are modify/delete on removed TurboQuant/tensor files — accept the deletions:
git rm vortex-turboquant/src/scalar_fns/decode.rs \
vortex-turboquant/src/scalar_fns/encode.rs \
vortex-turboquant/src/vector/normalize.rs \
vortex-tensor/src/encodings/turboquant/compress.rs \
vortex-tensor/src/encodings/turboquant/tests/compute.rs \
vortex-tensor/src/encodings/turboquant/tests/nullable.rs \
vortex-tensor/src/encodings/turboquant/tests/structural.rs \
vortex-tensor/src/scalar_fns/cosine_similarity.rs \
vortex-tensor/src/scalar_fns/inner_product.rs \
vortex-tensor/src/scalar_fns/l2_denorm.rs \
vortex-tensor/src/scalar_fns/l2_norm.rs \
vortex-tensor/src/scalar_fns/sorf_transform/mod.rs \
vortex-tensor/src/scalar_fns/sorf_transform/tests.rs \
vortex-tensor/src/utils.rs \
vortex-tensor/src/vector_search.rs
# The 10 vortex-array / encodings files carrying the `len` removal should merge cleanly.
git commit # completes the merge
git pushNote Given that #8377 removed the whole TurboQuant subsystem, those 15 files in this PR were only call-site updates for code that no longer exists — dropping them loses nothing. The meaningful change (the |
…ar-fn-array Signed-off-by: Claude <noreply@anthropic.com> # Conflicts: # vortex-tensor/src/encodings/turboquant/compress.rs # vortex-tensor/src/encodings/turboquant/tests/compute.rs # vortex-tensor/src/encodings/turboquant/tests/nullable.rs # vortex-tensor/src/encodings/turboquant/tests/structural.rs # vortex-tensor/src/scalar_fns/inner_product.rs # vortex-tensor/src/scalar_fns/sorf_transform/mod.rs # vortex-tensor/src/scalar_fns/sorf_transform/tests.rs # vortex-turboquant/src/scalar_fns/decode.rs # vortex-turboquant/src/scalar_fns/encode.rs # vortex-turboquant/src/vector/normalize.rs
Summary
This didn't need to take length because it already gets it from the array children.
API Changes
Removes
lenparameter.Testing
N/A