You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/02/03 15:49:49 UTC
[GitHub] [arrow] ritchie46 opened a new pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
ritchie46 opened a new pull request #9404:
URL: https://github.com/apache/arrow/pull/9404
This PR simplifies the `take_primitive` a little bit and increases performance by using the new `from_trusted_len_iterator` API.
```
take i32 512 time: [678.68 ns 692.27 ns 709.58 ns]
change: [-11.636% -10.615% -9.2359%] (p = 0.00 < 0.05)
Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
5 (5.00%) high mild
10 (10.00%) high severe
take i32 1024 time: [1.1393 us 1.1477 us 1.1637 us]
change: [-18.397% -17.829% -17.075%] (p = 0.00 < 0.05)
Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
1 (1.00%) low mild
3 (3.00%) high mild
5 (5.00%) high severe
take i32 nulls 512 time: [643.87 ns 644.11 ns 644.37 ns]
change: [-20.604% -20.531% -20.458%] (p = 0.00 < 0.05)
Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
2 (2.00%) low mild
6 (6.00%) high mild
5 (5.00%) high severe
take i32 nulls 1024 time: [1.1381 us 1.1384 us 1.1387 us]
change: [-19.419% -19.054% -18.753%] (p = 0.00 < 0.05)
Performance has improved.
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ritchie46 closed pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
ritchie46 closed pull request #9404:
URL: https://github.com/apache/arrow/pull/9404
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #9404:
URL: https://github.com/apache/arrow/pull/9404#issuecomment-772710405
in my experiments leading to #9301 I found no significant perf different between both variants and so did not consider an `unsafe` version, I admit that since the PR already offered a -30%, I also did not bother much ^_^
once all our implementations are `safe`, IMO we should get rid of the take options and potentially offer an `unsafe` version like you said. What imo is not cool is having an implementation whose safety depends on a parameter. :P
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #9404:
URL: https://github.com/apache/arrow/pull/9404#issuecomment-772696439
> Right, great work on that PR! :) Then I will close this one.
Thanks! :)
> I am also working on a perf increase of take_string. That one doesn't have collisions I presume?
No collisions, just go ahead and ping me if you need any help or a review. Note that there may be out-of-bound reads on that code. It would be awesome if those were taken care of :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ritchie46 commented on pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
ritchie46 commented on pull request #9404:
URL: https://github.com/apache/arrow/pull/9404#issuecomment-772693269
> Thanks @ritchie46 !
>
> Note that this collides with #9301 .
Right, great work on that PR! :) Then I will close this one.
I am also working on a perf increase of take_string. That one doesn't have collisions I presume?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9404:
URL: https://github.com/apache/arrow/pull/9404#issuecomment-772695067
https://issues.apache.org/jira/browse/ARROW-11482
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao edited a comment on pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
jorgecarleitao edited a comment on pull request #9404:
URL: https://github.com/apache/arrow/pull/9404#issuecomment-772710405
in my experiments leading to #9301 I found no significant perf different between both variants and so did not consider an `unsafe` version, I admit that since the PR already offered a -30%, I also did not bother much ^_^
once all our implementations are `sound`, IMO we should get rid of the take options and potentially offer an `unsafe` version like you said. What imo is not cool is having an implementation whose safety depends on a parameter. :P
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ritchie46 commented on pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
ritchie46 commented on pull request #9404:
URL: https://github.com/apache/arrow/pull/9404#issuecomment-772707038
> Note that there may be out-of-bound reads on that code. It would be awesome if those were taken care of :)
Ah, right so we want to `panic` if we have out-of-bounds access even if `check-bounds == false`. There are quite some cases where I know upfront that all my indexes will be valid. Should we also expose an unsafe variant of the take kernels? Especially in groupby and join operations their performance is paramount.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #9404:
URL: https://github.com/apache/arrow/pull/9404#issuecomment-772679863
Thanks @ritchie46 !
Note that this collides with #9301 .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ritchie46 commented on pull request #9404: ARROW-11482: [Rust] improve take kernel performance of PrimitiveArray
Posted by GitBox <gi...@apache.org>.
ritchie46 commented on pull request #9404:
URL: https://github.com/apache/arrow/pull/9404#issuecomment-772817451
> I admit that since the PR already offered a -30%, I also did not bother much ^_^
LOL.. That I can imagine.
> What imo is not cool is having an implementation whose safety depends on a parameter. :P
Agreed. I must say that I really notice the public API getting more idiomatic as well. It feels good to be able to use iterators for the fastest implementation!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org