-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add special implementation for zip for Utf8View/BinaryView scalars #8963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
70809ca to
d64eaf5
Compare
d64eaf5 to
1e05651
Compare
2006eff to
724157f
Compare
|
I have some decent speed-ups, see benchmarks. It would be great if somone could provide an initial review of this. |
724157f to
ba3c71a
Compare
|
@rluvaton Maybe you could have a look since you were the original author of this optimization? Thank you! |
|
Great job! Will try to review it today |
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR @mkleen -- could you please split out the benchmarks into a separate PR so it it easier to evaluate the performance difference due to this change?
94fd9a6 to
310a81a
Compare
310a81a to
918e2d0
Compare
Which issue does this PR close?
zipwith scalar for Utf8View #8724Rationale for this change
It's explained in the issue.
What changes are included in this PR?
This adds a special implementation for Utf8View/BinaryView scalars for zip based on the design from #8653. It also includes tests. Benchmarks are available here:
Are these changes tested?
Yes.
Are there any user-facing changes?
There is a new struct
ByteViewScalarImpl.Benchmarks
System: Apple M1 Max with 10 cores on macOS 26.1