Perf: remove extra allocations for hex operations - #24057
Merged
alamb merged 5 commits intoAug 15, 2026
Conversation
Rich-T-kid
force-pushed
the
rich-T-kid/avoid-hex-string-allocation
branch
from
August 3, 2026 05:27
edfa6cf to
15d22de
Compare
Rich-T-kid
marked this pull request as ready for review
August 3, 2026 05:31
Rich-T-kid
commented
Aug 3, 2026
Rich-T-kid
left a comment
Contributor
Author
There was a problem hiding this comment.
We may not need to deprecate this — just swapping out how DataFusion uses this internally may be enough, and it avoids a breaking API change.
Contributor
Author
local benchmarks show promising results
|
Rich-T-kid
force-pushed
the
rich-T-kid/avoid-hex-string-allocation
branch
from
August 3, 2026 05:55
15d22de to
67cb73d
Compare
Rich-T-kid
force-pushed
the
rich-T-kid/avoid-hex-string-allocation
branch
from
August 3, 2026 05:56
67cb73d to
4b880e1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #24057 +/- ##
==========================================
+ Coverage 80.88% 81.14% +0.26%
==========================================
Files 1101 1110 +9
Lines 375720 386233 +10513
Branches 375720 386233 +10513
==========================================
+ Hits 303895 313420 +9525
- Misses 53729 54353 +624
- Partials 18096 18460 +364 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Jefffrey
reviewed
Aug 13, 2026
Jefffrey
approved these changes
Aug 14, 2026
alamb
approved these changes
Aug 15, 2026
alamb
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me too -- thank you @Rich-T-kid and @Jefffrey
imtherealnaska
pushed a commit
to imtherealnaska/datafusion
that referenced
this pull request
Aug 16, 2026
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#23810. ## Rationale for this change The digest functions hex-encode their output by allocating one String per row and then copying each of those into the output array (apache#23810) <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> ## What changes are included in this PR? - This PR reduces per-row allocation and string-conversion overhead in the Spark crypto/hash paths by switching to the buffer-based hex encoder APIs. - Updates call sites to re-use vector buffers. <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? existing test cover behavior. <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? yes, `encode_bytes()` is being deprecated. This was apart of the public API <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. --> --------- Co-authored-by: rich-T-kid <richardbaah@MacBook-Air-de-Richard.local>
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
4 participants
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?
Rationale for this change
The digest functions hex-encode their output by allocating one String per row and then copying each of those into the output array (#23810)
What changes are included in this PR?
Are these changes tested?
existing test cover behavior.
Are there any user-facing changes?
yes,
encode_bytes()is being deprecated. This was apart of the public API