Skip to content
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

perf: Add criterion benchmark for xxhash64 function #560

Merged
merged 5 commits into from
Jun 13, 2024

Conversation

andygrove
Copy link
Member

Which issue does this PR close?

Related to #547

Rationale for this change

We suspect that we may have a performance issue in xxhash64, so the first step is to add a benchmark to evaluate current performance.

What changes are included in this PR?

New criterion benchmark

How are these changes tested?

Here are the results from Apple M3 Max.

hash/xxhash64/8192      time:   [891.80 µs 893.20 µs 895.25 µs]
Found 12 outliers among 100 measurements (12.00%)
  1 (1.00%) low mild
  5 (5.00%) high mild
  6 (6.00%) high severe

@@ -95,6 +96,16 @@ fn criterion_benchmark(c: &mut Criterion) {
});
},
);
group.bench_function(BenchmarkId::new("xxhash64", BATCH_SIZE), |b| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @andygrove. Could we extend this PR a bit and add a murmur3 hash benchmark as well?

In that way, I think we can get a sense of how slow xxhash64 is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@andygrove andygrove merged commit f95d386 into apache:main Jun 13, 2024
43 checks passed
@andygrove andygrove deleted the xxhash64-bench branch June 13, 2024 20:21
himadripal pushed a commit to himadripal/datafusion-comet that referenced this pull request Sep 7, 2024
* add benchmark for xxhash64

* remove new code

* add murmur3 bench

* format

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants