You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "mapleFU (via GitHub)" <gi...@apache.org> on 2023/12/26 11:31:46 UTC
[I] arrow::io::CompressedInputStream might allocate buffer to many times [arrow]
mapleFU opened a new issue, #39377:
URL: https://github.com/apache/arrow/issues/39377
### Describe the enhancement requested
in `::arrow::io::CompressedInputStream::Impl`, the `decompressed_buffer_` is initialized as 1MB, and would:
1. Call `Resize` to release the memory if possible ( https://github.com/apache/arrow/blob/main/cpp/src/arrow/io/compressed.cc#L303 )
2. Droped the memory when buffer exhausted ( https://github.com/apache/arrow/blob/main/cpp/src/arrow/io/compressed.cc#L324 )
Can we avoid re-allocating the buffer frequently, trying to keep 1MB data here?
### Component(s)
C++
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [C++] arrow::io::CompressedInputStream might allocate buffer to many times [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39377:
URL: https://github.com/apache/arrow/issues/39377#issuecomment-1869881645
```c++
Status Resize(const int64_t new_size, bool shrink_to_fit = true) override {
if (ARROW_PREDICT_FALSE(new_size < 0)) {
return Status::Invalid("Negative buffer resize: ", new_size);
}
uint8_t* ptr = mutable_data();
if (ptr && shrink_to_fit && new_size <= size_) {
// Buffer is non-null and is not growing, so shrink to the requested size without
// excess space.
int64_t new_capacity = bit_util::RoundUpToMultipleOf64(new_size);
if (capacity_ != new_capacity) {
// Buffer hasn't got yet the requested size.
RETURN_NOT_OK(pool_->Reallocate(capacity_, new_capacity, alignment_, &ptr));
data_ = ptr;
capacity_ = new_capacity;
}
} else {
RETURN_NOT_OK(Reserve(new_size));
}
size_ = new_size;
return Status::OK();
}
```
@felipecrv `Resize` has a `shrink_to_fit` option, wouldn't it free by `Reallocate`?
--
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: [I] [C++] arrow::io::CompressedInputStream might allocate buffer to many times [arrow]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on issue #39377:
URL: https://github.com/apache/arrow/issues/39377#issuecomment-1869918282
> I wonder if there is something to learn from the zero-copy stream design implemented in protocolbuffers
Do you mean GzipStream here: https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/io/gzip_stream.h ?
--
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: [I] [C++] arrow::io::CompressedInputStream might allocate buffer to many times [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on issue #39377:
URL: https://github.com/apache/arrow/issues/39377#issuecomment-1870538261
> @felipecrv Resize has a shrink_to_fit option, wouldn't it free by Reallocate?
You're right. The implementation is not a `realloc`, but a new allocation.
https://github.com/apache/arrow/blob/main/cpp/src/arrow/memory_pool.cc#L353
--
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: [I] [C++] arrow::io::CompressedInputStream might allocate buffer to many times [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on issue #39377:
URL: https://github.com/apache/arrow/issues/39377#issuecomment-1869630247
Wait... does `Resize` to a size smaller than the capacity of the buffer really frees memory?
I wonder if there is something to learn from the zero-copy stream design implemented in protocolbuffers
https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/io/zero_copy_stream.h
--
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: [I] [C++] arrow::io::CompressedInputStream might allocate buffer to many times [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on issue #39377:
URL: https://github.com/apache/arrow/issues/39377#issuecomment-2022482339
> You're right. The implementation is not a `realloc`, but a new allocation. https://github.com/apache/arrow/blob/main/cpp/src/arrow/memory_pool.cc#L353
Late reply to this, but you were looking at the system memory pool. Look for example at the mimalloc memory pool, and you'll see that it reallocates:
https://github.com/apache/arrow/blob/24feab091ab5a05b1cec234f51bd0223e2c41487/cpp/src/arrow/memory_pool.cc#L419-L420
Same for the jemalloc memory pool here:
https://github.com/apache/arrow/blob/24feab091ab5a05b1cec234f51bd0223e2c41487/cpp/src/arrow/memory_pool_jemalloc.cc#L111-L113
--
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: [I] [C++] arrow::io::CompressedInputStream might allocate buffer to many times [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou closed issue #39377: [C++] arrow::io::CompressedInputStream might allocate buffer to many times
URL: https://github.com/apache/arrow/issues/39377
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org