-
Notifications
You must be signed in to change notification settings - Fork 1k
Add benchmarks for FromIter (PrimitiveArray and BooleanArray) #8525
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
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.
Thanks for this @tobixdev -- looks great to me
// All ArrowPrimitiveType use the same implementation | ||
c.bench_function("Int64Array::from_iter", |b| { | ||
let values = gen_vector(1, ITER_LEN); | ||
b.iter(|| hint::black_box(Int64Array::from_iter(values.iter()))); |
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.
this looks similar to the existing benchmark here: https://github.com/apache/arrow-rs/blob/31ea84453b2f7ed7aa4e85825bd6cbf7ecd45f3a/arrow/benches/buffer_create.rs#L179-L178
Perhaps you could extend that benchmark rather than adding a new one?
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.
It certainly makes sense to add them to another benchmark. I wasn't aware of the suite at the top-level crate.
What do think about renaming array_from_vec
to array_from
and adding them there? I think this would also be related code but we wouldn't mix Array::from_xyz
with Buffer::from_xyz
. But I am also fine with adding it to buffer_create
.
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.
I've update it now to use array_from
. I can change it if you prefer buffer_create
.
74ecc68
to
5995fc3
Compare
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 @tobixdev
|
||
[[bench]] | ||
name = "array_from_vec" | ||
name = "array_from" |
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.
👍
Which issue does this PR close?
I want to see any performance regressions to
BooleanArray::from_iter
.Rationale for this change
Add microbenchmarks for observing the performance of
XYZArray::from_iter
.On my machine, executing the benchmarks back to back results in deviations within 1% .
What changes are included in this PR?
Only benchmarks
Are these changes tested?
Functionality is tested in the implementation file.
Are there any user-facing changes?
None