You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/08/21 16:13:37 UTC
[PR] Fix panic in comparison_kernel benchmarks [arrow-rs]
alamb opened a new pull request, #6284:
URL: https://github.com/apache/arrow-rs/pull/6284
# Which issue does this PR close?
Closes https://github.com/apache/arrow-rs/issues/6283
# Rationale for this change
Benchmark was panic'ing
Also having benchmarks in main ahead of a proposed code change makes it easier for my scripts to compare performance before/after code change
# What changes are included in this PR?
Also, adds benchmark from https://github.com/apache/arrow-rs/pull/6231 to make it easier to verify performance.
# Are there any user-facing changes?
No, this is development tooling only
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix panic in comparison_kernel benchmarks [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6284:
URL: https://github.com/apache/arrow-rs/pull/6284#issuecomment-2302482910
FYI @xinlifoobar
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix panic in comparison_kernel benchmarks [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #6284:
URL: https://github.com/apache/arrow-rs/pull/6284
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix panic in comparison_kernel benchmarks [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6284:
URL: https://github.com/apache/arrow-rs/pull/6284#issuecomment-2302938601
Thank you @appletreeisyellow and @wiedld for the reviews
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix panic in comparison_kernel benchmarks [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6284:
URL: https://github.com/apache/arrow-rs/pull/6284#discussion_r1725367870
##########
arrow/benches/comparison_kernels.rs:
##########
@@ -240,10 +241,18 @@ fn add_benchmark(c: &mut Criterion) {
b.iter(|| bench_like_utf8view_scalar(&string_view_left, "%xxxx"))
});
- c.bench_function("like_utf8view scalar starts with", |b| {
+ c.bench_function("like_utf8view scalar starts with less than 4 bytes", |b| {
b.iter(|| bench_like_utf8view_scalar(&string_view_left, "xxxx%"))
});
+ c.bench_function("like_utf8view scalar starts with more than 4 bytes", |b| {
+ b.iter(|| bench_like_utf8view_scalar(&string_view_left, "xxxxxx%"))
+ });
+
+ c.bench_function("like_utf8view scalar starts with more than 12 bytes", |b| {
Review Comment:
I also added a second function for more than 12 bytes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix panic in comparison_kernel benchmarks [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6284:
URL: https://github.com/apache/arrow-rs/pull/6284#discussion_r1725453967
##########
arrow/benches/comparison_kernels.rs:
##########
@@ -192,8 +192,20 @@ fn add_benchmark(c: &mut Criterion) {
})
});
- c.bench_function("eq scalar StringViewArray", |b| {
- b.iter(|| eq(&scalar, &string_view_left).unwrap())
Review Comment:
the panic came from this line which passes a scalar of type `StringArray` to a comparison kernel with `StringViewArray` -- the fix was to make the appropriate scalar type with `let string_view_scalar = StringViewArray::new_scalar("xxxx");`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org