You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Dandandan (via GitHub)" <gi...@apache.org> on 2025/04/18 12:47:48 UTC
[PR] Improve performance of interleave_primitive [arrow-rs]
Dandandan opened a new pull request, #7420:
URL: https://github.com/apache/arrow-rs/pull/7420
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
```
Benchmarking interleave i32(0.0) 100 [0..100, 100..230, 450..1000]: Warming up fBenchmarking interleave i32(0.0) 100 [0..100, 100..230, 450..1000]: Collecting 1interleave i32(0.0) 100 [0..100, 100..230, 450..1000]
time: [215.82 ns 217.15 ns 218.45 ns]
change: [-7.3367% -6.7273% -6.1944%] (p = 0.00 < 0.05)
Performance has improved.
Benchmarking interleave i32(0.0) 400 [0..100, 100..230, 450..1000]: Warming up fBenchmarking interleave i32(0.0) 400 [0..100, 100..230, 450..1000]: Collecting 1interleave i32(0.0) 400 [0..100, 100..230, 450..1000]
time: [444.54 ns 444.68 ns 444.85 ns]
change: [-13.019% -12.909% -12.808%] (p = 0.00 < 0.05)
Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
5 (5.00%) high mild
5 (5.00%) high severe
Benchmarking interleave i32(0.0) 1024 [0..100, 100..230, 450..1000]: Warming up Benchmarking interleave i32(0.0) 1024 [0..100, 100..230, 450..1000]: Collecting interleave i32(0.0) 1024 [0..100, 100..230, 450..1000]
time: [971.37 ns 971.67 ns 972.01 ns]
change: [-16.874% -16.323% -16.006%] (p = 0.00 < 0.05)
Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
4 (4.00%) high mild
4 (4.00%) high severe
Benchmarking interleave i32(0.0) 1024 [0..100, 100..230, 450..1000, 0..1000]: WaBenchmarking interleave i32(0.0) 1024 [0..100, 100..230, 450..1000, 0..1000]: CoBenchmarking interleave i32(0.0) 1024 [0..100, 100..230, 450..1000, 0..1000]: Aninterleave i32(0.0) 1024 [0..100, 100..230, 450..1000, 0..1000]
time: [980.50 ns 981.83 ns 983.20 ns]
change: [-15.619% -15.490% -15.359%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe
```
Closes #.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please call them out.
-->
--
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] [MINOR] Improve performance of interleave_primitive (-15% - 45%) / interleave_bytes (-10-25%) [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420#issuecomment-2815846332
> > Thanks for checking! I wonder, is this screenshot from Compiler Explorer or a local tool?
>
> It's [Beyond Compare](https://www.scootersoftware.com) but you can use your diff tool of choice with the two .txt dumps of the machine code. I use [cargo-show-asm](https://github.com/pacak/cargo-show-asm) to generate the relevant snippets of code.
>
> For example, `cargo asm -p arrow-select --lib --simplify` shows all the functions, and then I look for `interleave_primitive`. It was function 934 so I did `cargo asm -p arrow-select --lib --simplify 934 > ~/Desktop/pr.txt` to get the pr.txt file. You may need to add the `--context` argument if your function calls out to other functions, or look for functions that call your function if it's been inlined.
>
> I don't find that Compiler Explorer generates representative code for real projects. Putting snippets in there often doesn't reflect what the compiler does with large projects with complex CFG DAGs, external crates, LTO, and inlined functions.
Ah nice, thanks for the overview! I'll try might try that in the future.
--
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] Improve performance of interleave_primitive (-15% - 45%) / interleave_bytes (-10-25%) [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan merged PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420
--
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] [MINOR] Improve performance of interleave_primitive (-15%) [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420#issuecomment-2815671359
> Awesome! Makes me want to look at other places we're using `Vec::push`. I dug into the generated code to see what's going on. In the tight loop a big win is likely not having to jump out to `RawVec::grow_one`. Nice find! <img alt="Screenshot 2025-04-18 at 10 32 46 AM" width="1131" src="https://private-user-images.githubusercontent.com/6855576/435219477-db0391da-717f-4d06-994e-26fe21358aba.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDQ5ODcyMzQsIm5iZiI6MTc0NDk4NjkzNCwicGF0aCI6Ii82ODU1NTc2LzQzNTIxOTQ3Ny1kYjAzOTFkYS03MTdmLTRkMDYtOTk0ZS0yNmZlMjEzNThhYmEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDQxOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTA0MThUMTQzNTM0WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MjI4ZDcyMWRmNjI1MDIzYmEwMDcxZDdmNDFjMjFjM2M4NTlhOGVkYWM4ZWJhZGMxZmVhYzE3NjQ1OTAzNTUzNSZYLUFtei
1TaWduZWRIZWFkZXJzPWhvc3QifQ.-2aVPfXMDDWEhrTZ7GzPVyDUgAskZ6QVkVk-4-2uPkw"> [pr.txt](https://github.com/user-attachments/files/19813348/pr.txt) [main.txt](https://github.com/user-attachments/files/19813349/main.txt)
Thanks for checking! I wonder, is this screenshot from Compiler Explorer or a local tool?
--
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] [MINOR] Improve performance of interleave_primitive (-15%) / interleave_bytes (-30-40%) [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420#issuecomment-2815679365
I also pushed an improvement for `interleave_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] [MINOR] Improve performance of interleave_primitive (-15%) / interleave_bytes (-30-40%) [arrow-rs]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420#issuecomment-2815768881
> Thanks for checking! I wonder, is this screenshot from Compiler Explorer or a local tool?
It's [Beyond Compare](https://www.scootersoftware.com) but you can use your diff tool of choice with the two .txt dumps of the machine code. I use [cargo-show-asm](https://github.com/pacak/cargo-show-asm) to generate the relevant snippets of code.
For example, `cargo asm -p arrow-select --lib --simplify` shows all the functions, and then I look for `interleave_primitive`. It was function 934 so I did `cargo asm -p arrow-select --lib --simplify 934 > ~/Desktop/pr.txt` to get the pr.txt file. You may need to add the `--context` argument if your function calls it other functions, or look for functions that call your function if it's been inlined.
--
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] [MINOR] Improve performance of interleave_primitive (-15%) / interleave_bytes (-30-40%) [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420#discussion_r2050804471
##########
arrow-select/src/interleave.rs:
##########
@@ -170,14 +169,14 @@ fn interleave_bytes<T: ByteArrayType>(
let interleaved = Interleave::<'_, GenericByteArray<T>>::new(values, indices);
let mut capacity = 0;
- let mut offsets = BufferBuilder::<T::Offset>::new(indices.len() + 1);
- offsets.append(T::Offset::from_usize(0).unwrap());
- for (a, b) in indices {
+ let mut offsets = Vec::with_capacity(indices.len() + 1);
Review Comment:
`Vec` and `extend` generates better code.
There is probably some other places this pattern can be applied as well @mbutrovich
--
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] [MINOR] Improve performance of interleave_primitive (-15% - 45%) / interleave_bytes (-10-25%) [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420#discussion_r2050842953
##########
arrow-select/src/interleave.rs:
##########
@@ -132,12 +132,11 @@ impl<'a, T: Array + 'static> Interleave<'a, T> {
let nulls = match has_nulls {
true => {
- let mut builder = NullBufferBuilder::new(indices.len());
- for (a, b) in indices {
- let v = arrays[*a].is_valid(*b);
- builder.append(v)
- }
- builder.finish()
+ let nulls = BooleanBuffer::collect_bool(indices.len(), |i| {
Review Comment:
This is a pattern that comes up often as well
--
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] Improve performance of interleave_primitive (-15% - 45%) / interleave_bytes (-10-25%) [arrow-rs]
Posted by "pacak (via GitHub)" <gi...@apache.org>.
pacak commented on PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420#issuecomment-2819160136
> For example, `cargo asm -p arrow-select --lib --simplify` shows all the functions, and then I look for `interleave_primitive`. It was function 934 so I did `cargo asm -p arrow-select --lib --simplify 934`
If you know the name of the function or even part of it - you can specify all of it or parts as well: `cargo asm -p arrow-select --lib --simplify interleave_pr`, it should print only names that contain this substring - it should be easier to find the name you want since there will be less output. And then you can pass the number along with the name: `cargo asm -p arrow-select --lib --simplify interleave_pr 934`
--
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] Improve performance of interleave_primitive (-15% - 45%) / interleave_bytes (-10-25%) [arrow-rs]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on PR #7420:
URL: https://github.com/apache/arrow-rs/pull/7420#issuecomment-2819198978
> > For example, `cargo asm -p arrow-select --lib --simplify` shows all the functions, and then I look for `interleave_primitive`. It was function 934 so I did `cargo asm -p arrow-select --lib --simplify 934`
>
> If you know the name of the function or even part of it - you can specify all of it or parts as well: `cargo asm -p arrow-select --lib --simplify interleave_pr`, it should print only names that contain this substring - it should be easier to find the name you want since there will be less output. And then you can pass the number along with the name: `cargo asm -p arrow-select --lib --simplify interleave_pr 934`
TY @pacak !
--
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