-
Notifications
You must be signed in to change notification settings - Fork 907
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
Benchmark for strings::repeat_strings
APIs
#8589
Benchmark for strings::repeat_strings
APIs
#8589
Conversation
I just did a lot of cleanup for this PR. Now the code should look very clean for reviewing. |
strings::repeat_strings
[skip ci]strings::repeat_strings
APIs
Rerun tests. |
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.
lgtm 👍
int const min_rowlen = 1 << 4; | ||
int const max_rowlen = 1 << 8; |
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.
should these be str_len?
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.
Sure. Row length here is also string length.
Update: Done.
@ttnghia this status of this PR is conflicted. The target branch is still 21.08 but the project board says 21.10 (I made the latter change when it was not approved). Now that it is approved, if you are ready to merge you can go ahead and merge it into 21.08 as long as that is done before code freeze. But please move it to the 21.08 project board if you do so. Otherwise please change the target branch to 21.10 by clicking "edit" next to the title. |
Sorry for the confusion. I'm merging it to 21.08. |
@gpucibot merge |
This PR implemented benchmarks for the string APIs
repeat_strings
. The benchmark results listed below were generated from the current APIs and also from the same APIs but with some modifications.Note that this PR includes upstream code from #8561 thus the code from that PR is also listed here as "changed files".
Blocked by #8561.
Terms:
Benchmark results:
Machine:
Without any overflow checking:
With separate overflow checking:
With integrated overflow checking:
Performance graph: