(arrow-row) Replace BufferBuilder with Vec in decode_binary - #10851
Merged
Jefffrey merged 1 commit intoAug 26, 2026
Conversation
Contributor
|
run benchmark row_format |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing codex/arrow-rs-10245-decode-binary (1b9cd7a) to 2a82e59 (merge-base) diff Run configurationrun benchmark row_format
env:
BENCH_FILTER: "convert_rows"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Jefffrey
approved these changes
Aug 26, 2026
Contributor
|
thanks @zhangxinyao88 |
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.
Which issue does this PR close?
OffsetBufferBuilder/BufferBuilderusage withVec, when possible #10245.Rationale for this change
This replaces one of the remaining
BufferBuildercallsites listed in #10245. UsingVecfor the offsets avoids the builder overhead while preserving the resultingScalarBuffer.What changes are included in this PR?
decode_binarynow constructs its offsets withVec<I>instead ofBufferBuilder<I>.Are these changes tested?
Yes:
cargo fmt --all -- --checkcargo test -p arrow-row— 95 unit tests and 8 doc tests passedcargo clippy -p arrow-row --all-targets --all-features -- -D warningsExisting round-trip tests cover the changed path, so no new tests were added.
Benchmark:
convert_rows 4096 string(100, 0)mainCriterion reported a 4.4% improvement.
Are there any user-facing changes?
No.
AI assistance
Codex was used to identify the callsite, prepare the focused refactor, run validation, and draft this description.