[GitHub] [arrow] sunchao commented on a change in pull request #9044: ARROW-11045: [Rust] Fix performance issues of allocator

[GitBox <gi...@apache.org>]

sunchao commented on a change in pull request #9044:
URL: https://github.com/apache/arrow/pull/9044#discussion_r550906779



##########
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:
       Is there any reason to change the API in this PR? don't see how it could be related to the performance issue.
   Also better to split it to make PR easier to review. We also need to update the doc.

##########
File path: rust/arrow/src/buffer.rs
##########
@@ -774,16 +756,16 @@ impl MutableBuffer {
     /// `new_len` will be zeroed out.
     ///
     /// If `new_len` is less than `len`, the buffer will be truncated.
-    pub fn resize(&mut self, new_len: usize) {
+    pub fn resize(&mut self, new_len: usize, value: u8) {

Review comment:
       we could have another method with `value` = 0 since this seems to be the dominate use case (it might enable future optimizations for the special case 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org