You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by uw...@apache.org on 2018/11/03 10:16:22 UTC
[arrow] branch master updated: ARROW-3610: [C++] Add interface to
turn stl_allocator into arrow::MemoryPool
This is an automated email from the ASF dual-hosted git repository.
uwe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 010fad2 ARROW-3610: [C++] Add interface to turn stl_allocator into arrow::MemoryPool
010fad2 is described below
commit 010fad206298723f45b0f819364f6127510faf77
Author: Korn, Uwe <Uw...@blue-yonder.com>
AuthorDate: Sat Nov 3 11:06:53 2018 +0100
ARROW-3610: [C++] Add interface to turn stl_allocator into arrow::MemoryPool
First outcome from PyConDE collaboration with @wolfv
Author: Korn, Uwe <Uw...@blue-yonder.com>
Closes #2837 from xhochy/ARROW-3610 and squashes the following commits:
0dfe4a4a <Korn, Uwe> Remove test that relied on DCHECK
48752fac <Korn, Uwe> Add atomic header at the right place
1a4fb01d <Korn, Uwe> ARROW-3610: Add interface to turn stl_allocator into arrow::MemoryPool
---
cpp/src/arrow/allocator-test.cc | 34 +++++++++++++++++++------------
cpp/src/arrow/allocator.h | 43 +++++++++++++++++++++++++++++++++++++++
cpp/src/arrow/memory_pool-test.cc | 14 -------------
cpp/src/arrow/memory_pool.cc | 30 ++-------------------------
cpp/src/arrow/memory_pool.h | 30 +++++++++++++++++++++++++++
5 files changed, 96 insertions(+), 55 deletions(-)
diff --git a/cpp/src/arrow/allocator-test.cc b/cpp/src/arrow/allocator-test.cc
index bdae9b9..cdffbd7 100644
--- a/cpp/src/arrow/allocator-test.cc
+++ b/cpp/src/arrow/allocator-test.cc
@@ -23,9 +23,30 @@
#include "arrow/allocator.h"
#include "arrow/memory_pool.h"
+#include "arrow/test-util.h"
namespace arrow {
+TEST(STLMemoryPool, Base) {
+ std::allocator<uint8_t> allocator;
+ STLMemoryPool<std::allocator<uint8_t>> pool(allocator);
+
+ uint8_t* data = nullptr;
+ ASSERT_OK(pool.Allocate(100, &data));
+ ASSERT_EQ(pool.max_memory(), 100);
+ ASSERT_EQ(pool.bytes_allocated(), 100);
+ ASSERT_NE(data, nullptr);
+
+ ASSERT_OK(pool.Reallocate(100, 150, &data));
+ ASSERT_EQ(pool.max_memory(), 150);
+ ASSERT_EQ(pool.bytes_allocated(), 150);
+
+ pool.Free(data, 150);
+
+ ASSERT_EQ(pool.max_memory(), 150);
+ ASSERT_EQ(pool.bytes_allocated(), 0);
+}
+
TEST(stl_allocator, MemoryTracking) {
auto pool = default_memory_pool();
stl_allocator<uint64_t> alloc;
@@ -45,19 +66,6 @@ TEST(stl_allocator, TestOOM) {
ASSERT_THROW(alloc.allocate(to_alloc), std::bad_alloc);
}
-TEST(stl_allocator, FreeLargeMemory) {
- stl_allocator<uint8_t> alloc;
-
- uint8_t* data = alloc.allocate(100);
-
-#ifndef NDEBUG
- EXPECT_DEATH(alloc.deallocate(data, 120),
- ".*Check failed:.* allocation counter became negative");
-#endif
-
- alloc.deallocate(data, 100);
-}
-
TEST(stl_allocator, MaxMemory) {
auto pool = default_memory_pool();
diff --git a/cpp/src/arrow/allocator.h b/cpp/src/arrow/allocator.h
index c7780f1..144ba57 100644
--- a/cpp/src/arrow/allocator.h
+++ b/cpp/src/arrow/allocator.h
@@ -18,6 +18,7 @@
#ifndef ARROW_ALLOCATOR_H
#define ARROW_ALLOCATOR_H
+#include <algorithm>
#include <cstddef>
#include <memory>
#include <utility>
@@ -85,6 +86,48 @@ class stl_allocator {
MemoryPool* pool_;
};
+template <typename Allocator = std::allocator<uint8_t>>
+class STLMemoryPool : public MemoryPool {
+ public:
+ explicit STLMemoryPool(const Allocator& alloc) : alloc_(alloc) {}
+
+ Status Allocate(int64_t size, uint8_t** out) override {
+ try {
+ *out = alloc_.allocate(size);
+ } catch (std::bad_alloc& e) {
+ return Status::OutOfMemory(e.what());
+ }
+ stats_.UpdateAllocatedBytes(size);
+ return Status::OK();
+ }
+
+ Status Reallocate(int64_t old_size, int64_t new_size, uint8_t** ptr) override {
+ uint8_t* old_ptr = *ptr;
+ try {
+ *ptr = alloc_.allocate(new_size);
+ } catch (std::bad_alloc& e) {
+ return Status::OutOfMemory(e.what());
+ }
+ memcpy(*ptr, old_ptr, std::min(old_size, new_size));
+ alloc_.deallocate(old_ptr, old_size);
+ stats_.UpdateAllocatedBytes(new_size - old_size);
+ return Status::OK();
+ }
+
+ void Free(uint8_t* buffer, int64_t size) override {
+ alloc_.deallocate(buffer, size);
+ stats_.UpdateAllocatedBytes(-size);
+ }
+
+ int64_t bytes_allocated() const override { return stats_.bytes_allocated(); }
+
+ int64_t max_memory() const override { return stats_.max_memory(); }
+
+ private:
+ Allocator alloc_;
+ internal::MemoryPoolStats stats_;
+};
+
template <class T1, class T2>
bool operator==(const stl_allocator<T1>& lhs, const stl_allocator<T2>& rhs) noexcept {
return lhs.pool() == rhs.pool();
diff --git a/cpp/src/arrow/memory_pool-test.cc b/cpp/src/arrow/memory_pool-test.cc
index ad60a93..5a11073 100644
--- a/cpp/src/arrow/memory_pool-test.cc
+++ b/cpp/src/arrow/memory_pool-test.cc
@@ -45,20 +45,6 @@ TEST_F(TestDefaultMemoryPool, Reallocate) { this->TestReallocate(); }
// googletest documentation
#if !(defined(ARROW_VALGRIND) || defined(ADDRESS_SANITIZER))
-TEST(DefaultMemoryPoolDeathTest, FreeLargeMemory) {
- MemoryPool* pool = default_memory_pool();
-
- uint8_t* data;
- ASSERT_OK(pool->Allocate(100, &data));
-
-#ifndef NDEBUG
- EXPECT_DEATH(pool->Free(data, 120),
- ".*Check failed:.* allocation counter became negative");
-#endif
-
- pool->Free(data, 100);
-}
-
TEST(DefaultMemoryPoolDeathTest, MaxMemory) {
MemoryPool* pool = default_memory_pool();
uint8_t* data1;
diff --git a/cpp/src/arrow/memory_pool.cc b/cpp/src/arrow/memory_pool.cc
index 84c1853..4947f6a 100644
--- a/cpp/src/arrow/memory_pool.cc
+++ b/cpp/src/arrow/memory_pool.cc
@@ -88,32 +88,6 @@ MemoryPool::~MemoryPool() {}
int64_t MemoryPool::max_memory() const { return -1; }
///////////////////////////////////////////////////////////////////////
-// Helper tracking memory statistics
-
-class MemoryPoolStats {
- public:
- MemoryPoolStats() : bytes_allocated_(0), max_memory_(0) {}
-
- int64_t max_memory() const { return max_memory_.load(); }
-
- int64_t bytes_allocated() const { return bytes_allocated_.load(); }
-
- inline void UpdateAllocatedBytes(int64_t diff) {
- auto allocated = bytes_allocated_.fetch_add(diff) + diff;
- DCHECK_GE(allocated, 0) << "allocation counter became negative";
- // "maximum" allocated memory is ill-defined in multi-threaded code,
- // so don't try to be too rigorous here
- if (diff > 0 && allocated > max_memory_) {
- max_memory_ = allocated;
- }
- }
-
- protected:
- std::atomic<int64_t> bytes_allocated_;
- std::atomic<int64_t> max_memory_;
-};
-
-///////////////////////////////////////////////////////////////////////
// Default MemoryPool implementation
class DefaultMemoryPool : public MemoryPool {
@@ -174,7 +148,7 @@ class DefaultMemoryPool : public MemoryPool {
int64_t max_memory() const override { return stats_.max_memory(); }
private:
- MemoryPoolStats stats_;
+ internal::MemoryPoolStats stats_;
};
MemoryPool* default_memory_pool() {
@@ -247,7 +221,7 @@ class ProxyMemoryPool::ProxyMemoryPoolImpl {
private:
MemoryPool* pool_;
- MemoryPoolStats stats_;
+ internal::MemoryPoolStats stats_;
};
ProxyMemoryPool::ProxyMemoryPool(MemoryPool* pool) {
diff --git a/cpp/src/arrow/memory_pool.h b/cpp/src/arrow/memory_pool.h
index 0ee46a3..37f31cc 100644
--- a/cpp/src/arrow/memory_pool.h
+++ b/cpp/src/arrow/memory_pool.h
@@ -18,6 +18,7 @@
#ifndef ARROW_MEMORY_POOL_H
#define ARROW_MEMORY_POOL_H
+#include <atomic>
#include <cstdint>
#include <memory>
@@ -25,6 +26,35 @@
namespace arrow {
+namespace internal {
+
+///////////////////////////////////////////////////////////////////////
+// Helper tracking memory statistics
+
+class MemoryPoolStats {
+ public:
+ MemoryPoolStats() : bytes_allocated_(0), max_memory_(0) {}
+
+ int64_t max_memory() const { return max_memory_.load(); }
+
+ int64_t bytes_allocated() const { return bytes_allocated_.load(); }
+
+ inline void UpdateAllocatedBytes(int64_t diff) {
+ auto allocated = bytes_allocated_.fetch_add(diff) + diff;
+ // "maximum" allocated memory is ill-defined in multi-threaded code,
+ // so don't try to be too rigorous here
+ if (diff > 0 && allocated > max_memory_) {
+ max_memory_ = allocated;
+ }
+ }
+
+ protected:
+ std::atomic<int64_t> bytes_allocated_;
+ std::atomic<int64_t> max_memory_;
+};
+
+} // namespace internal
+
class Status;
/// Base class for memory allocation.