GH-43444: [C++] Add benchmark for binary view builder #43445
Conversation
|
|
| @@ -26,6 +26,7 @@ | |||
| namespace arrow::util { | |||
|
|
|||
| inline BinaryViewType::c_type ToInlineBinaryView(const void* data, int32_t size) { | |||
| assert(size <= BinaryViewType::kInlineSize); | |||
There was a problem hiding this comment.
https://godbolt.org/z/q8xjb8Ehe
From here, COMPILER ASSUMER might generate better code in C++
There was a problem hiding this comment.
Which I think we be assumed when this is inlined from the if below.
|
@felipecrv Would you mind take a look? |
mapleFU
changed the title
github-actions
bot
added
awaiting committer review
| return ToInlineBinaryView(data, size); | ||
| } | ||
|
|
||
| inline BinaryViewType::c_type ToLargeBinaryView(const void* data, int32_t size, |
There was a problem hiding this comment.
nit: call this ToNonInlineBinaryView to not confuse people that might think a LARGE_BINARY_VIEW type exists. [I misread it like this at first :)]
| @@ -26,6 +26,7 @@ | |||
| namespace arrow::util { | |||
|
|
|||
| inline BinaryViewType::c_type ToInlineBinaryView(const void* data, int32_t size) { | |||
| assert(size <= BinaryViewType::kInlineSize); | |||
There was a problem hiding this comment.
Which I think we be assumed when this is inlined from the if below.
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
github-actions
bot
added
awaiting changes
github-actions
bot
added
awaiting change review
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit c6be2df. There was 1 benchmark result indicating a performance regression:
The full Conbench report has more details. It also includes information about 29 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
Adding benchmark for BinaryViewBuilder
What changes are included in this PR?
Adding benchmark for BinaryViewBuilder
Are these changes tested?
No
Are there any user-facing changes?
No