jorgecarleitao commented on a change in pull request #9044:
URL: https://github.com/apache/arrow/pull/9044#discussion_r550908549
##########
File path: rust/arrow/src/buffer.rs
##########
@@ -756,15 +738,15 @@ impl MutableBuffer {
/// also ensure the new capacity will be a multiple of 64 bytes.
///
/// Returns the new capacity for this buffer.
- pub fn reserve(&mut self, capacity: usize) -> usize {
- if capacity > self.capacity {
- let new_capacity = bit_util::round_upto_multiple_of_64(capacity);
- let new_capacity = cmp::max(new_capacity, self.capacity * 2);
+ pub fn reserve(&mut self, additional: usize) {
Review comment:
I agree that it makes it more difficult. The signature is related to
performance, though:
It was difficult to reason with `reserve(new_len)` when `Vec::reserve` and
`RawVec::reserve` uses `additional` (and I was basing this PR on that). This PR
fixes a bug in one `Builder` that was calling `reserve(1)` in `append`, when it
should be calling `reserve(self.len + 1)`. I also spent 4hs tracking a bug
because of this difference (I was using `reserve(additional)` when its
signature was `reserve(new_len)`.
I can try to split that to another PR, though. IMO this PR will still need
to be based on that 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:
[email protected]
