-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Improve PrimitiveArray::from_iter perf
#9294
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
Conversation
PrimitiveArray::from_iterPrimitiveArray::from_iter perf
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.
Looks like a good improvement to me.
|
run benchmark arrow_statistics |
Sorry I think the VM runner got rebooted / wasn't working. I restarted it and now the queue is good |
|
show benchmark queue |
|
🤖 Hi @alamb, you asked to view the benchmark queue (#9294 (comment)).
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
run benchmark arrow_statistics |
|
🤖 |
|
🤖: Benchmark completed Details
|
|
Those are some pretty sweet results |
…om Vec and `from_trusted_len_iter` (#9299) # Which issue does this PR close? - part of #9298 # Rationale for this change While reviewing #9294 from @Dandandan I noticed some other places where we can avoid making ArrayData and thus save some allocations (and `unsafe`) I don't expect this to make a huge performance difference, but every little allocation helps, and I think the change is justified simply from the perspective of avoiding some more `unsafe` # What changes are included in this PR? Construct primitive arrays directly # Are these changes tested? By existing CI # Are there any user-facing changes? <!-- 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 call them out. -->
Which issue does this PR close?
Rationale for this change
Speeds up
from_iter.This speeds up creation for statistics if all values are present (common case):
What changes are included in this PR?