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 2020/12/28 17:21:06 UTC
[GitHub] [arrow] jorgecarleitao opened a new pull request #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
jorgecarleitao opened a new pull request #9032:
URL: https://github.com/apache/arrow/pull/9032
This bench compares the behavior of `MutableBuffer` vs allocating `Vec<u8>` and using `extend_from_slice`.
On my computer:
```
mutable time: [579.24 us 580.21 us 581.34 us]
mutable prepared time: [614.98 us 616.15 us 617.42 us]
from_slice time: [1.2945 ms 1.3262 ms 1.3607 ms]
from_slice prepared time: [1.0161 ms 1.0472 ms 1.0881 ms]
```
I.e. growing a `MutableBuffer` seems to be 2x faster than creating a `Vec` and using `Buffer::from` to convert it to a `Buffer`.
It is odd that creating a buffer with the correct capacity takes longer, though. Any ideas @jhorstmann , @Dandandan ?
----------------------------------------------------------------
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 #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
jorgecarleitao edited a comment on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-752045697
just FYI, I found one reason. `alloc_zeroed` is expensive. I am preparing a PR where this is addressed. There is some code atm that relies on `MutableBuffer::reserve` and `MutableBuffer::new` to allocate zeros (and not undefined), which I need to handle.
----------------------------------------------------------------
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] Dandandan commented on pull request #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
Dandandan commented on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-751841922
I am not sure what is causing the difference, I would expect it to be faster just as `from_slice prepared` is faster.
The main difference is that initializing with capacity `0` uses `memory::allocate_aligned` while the other uses a few calls to `memory::reallocate`. I guess the first is (quite) a bit slower somehow?
----------------------------------------------------------------
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] codecov-io commented on pull request #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-753283834
# [Codecov](https://codecov.io/gh/apache/arrow/pull/9032?src=pr&el=h1) Report
> Merging [#9032](https://codecov.io/gh/apache/arrow/pull/9032?src=pr&el=desc) (7a0fa18) into [master](https://codecov.io/gh/apache/arrow/commit/51672b28e97f19f70de0f0a8800c40ee9bb939d3?el=desc) (51672b2) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/arrow/pull/9032?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9032 +/- ##
=======================================
Coverage 82.61% 82.61%
=======================================
Files 202 202
Lines 50048 50048
=======================================
Hits 41347 41347
Misses 8701 8701
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9032?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9032?src=pr&el=footer). Last update [51672b2...7a0fa18](https://codecov.io/gh/apache/arrow/pull/9032?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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 #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-751811466
https://issues.apache.org/jira/browse/ARROW-11048
----------------------------------------------------------------
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] jhorstmann commented on pull request #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
jhorstmann commented on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-751845418
I think `Buffer::from` currently does another copy of the vec contents because it does not take ownership and because of the current alignment and padding requirements. We probably could add a zero-copy `Buffer::from_vec` method if we loosen those restrictions. Another interesting benchmark would be another version of `from_slice` that returns a `Vec` instead of `Buffer`, to see whether the extend/reallocate logic itself could be optimized.
----------------------------------------------------------------
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 closed pull request #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
jorgecarleitao closed pull request #9032:
URL: https://github.com/apache/arrow/pull/9032
----------------------------------------------------------------
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] alamb commented on pull request #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-752956532
The full set of Rust CI tests did not run on this PR :(
Can you please rebase this PR against [apache/master](https://github.com/apache/arrow) to pick up the changes in https://github.com/apache/arrow/pull/9056 so that they do?
I apologize for the inconvenience.
----------------------------------------------------------------
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 #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-753449574
Closing as this code was merged as part of another PR
----------------------------------------------------------------
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] alamb commented on pull request #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-753304964
I believe these benches were already added as part of #8997 (which I found out while trying to merge this PR -- LOL)
----------------------------------------------------------------
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 #9032: ARROW-11048: [Rust] Added bench to MutableBuffer
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #9032:
URL: https://github.com/apache/arrow/pull/9032#issuecomment-752045697
just FYI, I found the reason. `alloc_zeroed` is expensive. I am preparing a PR where this is addressed. There is some code atm that relies on `MutableBuffer::reserve` and `MutableBuffer::new` to allocate zeros (and not undefined), which I need to handle.
----------------------------------------------------------------
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