You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@orc.apache.org by ga...@apache.org on 2026/05/12 02:24:23 UTC
(orc) branch main updated: ORC-2164: [C++] Add external-buffer support in DataBuffer lifecycle management
This is an automated email from the ASF dual-hosted git repository.
wgtmac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/orc.git
The following commit(s) were added to refs/heads/main by this push:
new fbc8c8771 ORC-2164: [C++] Add external-buffer support in DataBuffer lifecycle management
fbc8c8771 is described below
commit fbc8c8771864ce08127a843237534b4f50f3e256
Author: yonghao.fyh <yo...@alibaba-inc.com>
AuthorDate: Tue May 12 10:24:12 2026 +0800
ORC-2164: [C++] Add external-buffer support in DataBuffer lifecycle management
### What changes were proposed in this pull request?
This PR adds non-owning buffer support to DataBuffer so it can attach externally managed memory (via setData) without taking ownership of allocation/free, updates reserve/resize/destructor paths to be ownership-aware, and adds a focused unit test to verify that non-owning buffers keep external size semantics and do not free external memory during destruction.
### Why are the changes needed?
These changes are needed to safely integrate with external prebuffer cache scenarios where memory is managed outside ORC, avoiding unnecessary reallocations and preventing ownership/lifecycle mismatches.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: GPT-5.3-codex
Closes #2617 from lucasfang/mempool.
Authored-by: yonghao.fyh <yo...@alibaba-inc.com>
Signed-off-by: Gang Wu <us...@gmail.com>
---
c++/include/orc/MemoryPool.hh | 7 +++-
c++/src/MemoryPool.cc | 88 +++++++++++++++++++++++++++++++++++--------
c++/test/TestCache.cc | 37 ++++++++++++++++++
3 files changed, 116 insertions(+), 16 deletions(-)
diff --git a/c++/include/orc/MemoryPool.hh b/c++/include/orc/MemoryPool.hh
index a914e5f26..8a7d1003c 100644
--- a/c++/include/orc/MemoryPool.hh
+++ b/c++/include/orc/MemoryPool.hh
@@ -42,13 +42,15 @@ namespace orc {
uint64_t currentSize_;
// maximal capacity (actual allocated memory)
uint64_t currentCapacity_;
+ // whether this buffer owns memory lifecycle
+ bool ownBuffer_;
// not implemented
DataBuffer(DataBuffer& buffer);
DataBuffer& operator=(DataBuffer& buffer);
public:
- DataBuffer(MemoryPool& pool, uint64_t size = 0);
+ DataBuffer(MemoryPool& pool, uint64_t size = 0, bool ownBuf = true);
DataBuffer(DataBuffer<T>&& buffer) noexcept;
@@ -81,6 +83,9 @@ namespace orc {
void reserve(uint64_t size);
void resize(uint64_t size);
void zeroOut();
+
+ // Attach to an external raw buffer. bufSize is in bytes.
+ void setData(T* buf, size_t bufSize);
};
// Specializations for char
diff --git a/c++/src/MemoryPool.cc b/c++/src/MemoryPool.cc
index ed7fee737..43f5b6212 100644
--- a/c++/src/MemoryPool.cc
+++ b/c++/src/MemoryPool.cc
@@ -24,6 +24,7 @@
#include <string.h>
#include <cstdlib>
#include <iostream>
+#include <type_traits>
namespace orc {
@@ -52,10 +53,12 @@ namespace orc {
}
template <class T>
- DataBuffer<T>::DataBuffer(MemoryPool& pool, uint64_t newSize)
- : memoryPool_(pool), buf_(nullptr), currentSize_(0), currentCapacity_(0) {
- reserve(newSize);
- currentSize_ = newSize;
+ DataBuffer<T>::DataBuffer(MemoryPool& pool, uint64_t newSize, bool ownBuf)
+ : memoryPool_(pool), buf_(nullptr), currentSize_(0), currentCapacity_(0), ownBuffer_(ownBuf) {
+ if (ownBuffer_) {
+ reserve(newSize);
+ currentSize_ = newSize;
+ }
}
template <class T>
@@ -63,24 +66,34 @@ namespace orc {
: memoryPool_(buffer.memoryPool_),
buf_(buffer.buf_),
currentSize_(buffer.currentSize_),
- currentCapacity_(buffer.currentCapacity_) {
+ currentCapacity_(buffer.currentCapacity_),
+ ownBuffer_(buffer.ownBuffer_) {
buffer.buf_ = nullptr;
buffer.currentSize_ = 0;
buffer.currentCapacity_ = 0;
+ buffer.ownBuffer_ = true;
}
template <class T>
DataBuffer<T>::~DataBuffer() {
+ if (!ownBuffer_) {
+ return;
+ }
for (uint64_t i = currentSize_; i > 0; --i) {
(buf_ + i - 1)->~T();
}
if (buf_) {
+ static_assert(std::is_trivially_copyable<T>::value,
+ "Only trivially copyable type is supported for DataBuffer reserve");
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <class T>
void DataBuffer<T>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (currentSize_ > newSize) {
for (uint64_t i = currentSize_; i > newSize; --i) {
@@ -96,6 +109,9 @@ namespace orc {
template <class T>
void DataBuffer<T>::reserve(uint64_t newCapacity) {
+ if (!ownBuffer_) {
+ return;
+ }
if (newCapacity > currentCapacity_ || !buf_) {
if (buf_) {
T* buf_old = buf_;
@@ -114,6 +130,18 @@ namespace orc {
memset(buf_, 0, sizeof(T) * currentCapacity_);
}
+ template <class T>
+ void DataBuffer<T>::setData(T* buffer, size_t bufSize) {
+ if (ownBuffer_ && buf_) {
+ static_assert(std::is_trivially_copyable<T>::value,
+ "Only trivially copyable type is supported for DataBuffer reserve");
+ memoryPool_.free(reinterpret_cast<char*>(buf_));
+ }
+ ownBuffer_ = false;
+ buf_ = buffer;
+ currentSize_ = currentCapacity_ = static_cast<uint64_t>(bufSize / sizeof(T));
+ }
+
// Specializations for Int128
template <>
void DataBuffer<Int128>::zeroOut() {
@@ -126,13 +154,16 @@ namespace orc {
template <>
DataBuffer<char>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<char>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, newSize - currentSize_);
@@ -144,13 +175,16 @@ namespace orc {
template <>
DataBuffer<char*>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<char*>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, (newSize - currentSize_) * sizeof(char*));
@@ -162,13 +196,16 @@ namespace orc {
template <>
DataBuffer<double>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<double>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, (newSize - currentSize_) * sizeof(double));
@@ -180,13 +217,16 @@ namespace orc {
template <>
DataBuffer<float>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<float>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, (newSize - currentSize_) * sizeof(float));
@@ -198,13 +238,16 @@ namespace orc {
template <>
DataBuffer<int64_t>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<int64_t>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, (newSize - currentSize_) * sizeof(int64_t));
@@ -216,13 +259,16 @@ namespace orc {
template <>
DataBuffer<int32_t>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<int32_t>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, (newSize - currentSize_) * sizeof(int32_t));
@@ -234,13 +280,16 @@ namespace orc {
template <>
DataBuffer<int16_t>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<int16_t>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, (newSize - currentSize_) * sizeof(int16_t));
@@ -252,13 +301,16 @@ namespace orc {
template <>
DataBuffer<int8_t>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<int8_t>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, (newSize - currentSize_) * sizeof(int8_t));
@@ -270,13 +322,16 @@ namespace orc {
template <>
DataBuffer<uint64_t>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<uint64_t>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, (newSize - currentSize_) * sizeof(uint64_t));
@@ -288,13 +343,16 @@ namespace orc {
template <>
DataBuffer<unsigned char>::~DataBuffer() {
- if (buf_) {
+ if (ownBuffer_ && buf_) {
memoryPool_.free(reinterpret_cast<char*>(buf_));
}
}
template <>
void DataBuffer<unsigned char>::resize(uint64_t newSize) {
+ if (!ownBuffer_) {
+ return;
+ }
reserve(newSize);
if (newSize > currentSize_) {
memset(buf_ + currentSize_, 0, newSize - currentSize_);
diff --git a/c++/test/TestCache.cc b/c++/test/TestCache.cc
index 496ba3ec9..6aec05cf4 100644
--- a/c++/test/TestCache.cc
+++ b/c++/test/TestCache.cc
@@ -26,6 +26,22 @@
namespace orc {
+ class CountingMemoryPool : public MemoryPool {
+ public:
+ uint64_t allocCount = 0;
+ uint64_t freeCount = 0;
+
+ char* malloc(uint64_t size) override {
+ ++allocCount;
+ return static_cast<char*>(std::malloc(size));
+ }
+
+ void free(char* p) override {
+ ++freeCount;
+ std::free(p);
+ }
+ };
+
TEST(TestReadRangeCombiner, testBasics) {
ReadRangeCombiner combinator{0, 100};
/// Ranges with partial overlap and identical offsets
@@ -139,4 +155,25 @@ namespace orc {
slice = cache.read({20, 2});
assert_slice_equal(slice, "uv");
}
+
+ TEST(TestDataBuffer, testExternalBufferNonOwning) {
+ CountingMemoryPool pool;
+ char external[16] = {0};
+ uint64_t freeCountAfterSetData = 0;
+
+ {
+ DataBuffer<char> buffer(pool, 0);
+ buffer.setData(external, sizeof(external));
+ freeCountAfterSetData = pool.freeCount;
+
+ // Non-owning buffer should keep external size and ignore resize.
+ EXPECT_EQ(sizeof(external), buffer.size());
+ buffer.resize(8);
+ EXPECT_EQ(sizeof(external), buffer.size());
+ }
+
+ // setData may release previously owned internal memory, but destruction should not free
+ // external.
+ EXPECT_EQ(freeCountAfterSetData, pool.freeCount);
+ }
} // namespace orc