Vineyard fails to compile with Arrow 12.0.0

[raulcd]

Describe your problem

Vineyard fails with Arrow 12.0.0.

When trying to update hombrew-core for arrow 12.0.0 release vineyard fails with:

   /tmp/vineyard-20230503-55118-14bq3js/v6d-0.14.3/modules/basic/ds/arrow.cc:335:30: error: variable type 'memory::VineyardMemoryPool' is an abstract class
    memory::VineyardMemoryPool pool(client);
                               ^
  /opt/homebrew/include/arrow/memory_pool.h:141:19: note: unimplemented pure virtual method 'total_bytes_allocated' in 'VineyardMemoryPool'
    virtual int64_t total_bytes_allocated() const = 0;
                    ^
  /opt/homebrew/include/arrow/memory_pool.h:144:19: note: unimplemented pure virtual method 'num_allocations' in 'VineyardMemoryPool'
    virtual int64_t num_allocations() const = 0;
                    ^
  /tmp/vineyard-20230503-55118-14bq3js/v6d-0.14.3/modules/basic/ds/arrow.cc:456:30: error: variable type 'memory::VineyardMemoryPool' is an abstract class
    memory::VineyardMemoryPool pool(client);
                               ^
  /tmp/vineyard-20230503-55118-14bq3js/v6d-0.14.3/modules/basic/ds/arrow.cc:526:30: error: variable type 'memory::VineyardMemoryPool' is an abstract class
    memory::VineyardMemoryPool pool(client);
                               ^
  /tmp/vineyard-20230503-55118-14bq3js/v6d-0.14.3/modules/basic/ds/arrow.cc:595:30: error: variable type 'memory::VineyardMemoryPool' is an abstract class
    memory::VineyardMemoryPool pool(client);
                               ^
  /tmp/vineyard-20230503-55118-14bq3js/v6d-0.14.3/modules/basic/ds/arrow.cc:647:30: error: variable type 'memory::VineyardMemoryPool' is an abstract class
    memory::VineyardMemoryPool pool(client);

See the PR on homebrew-core: Homebrew/homebrew-core#129859
This is due to this new implemented feature on Arrow: apache/arrow#33731

---

If is is a bug report, to help us reproducing this bug, please provide information below:

Shown on the linked PR for homebrew-core. To reproduce use arrow 12.0.0.

What is the problem:

Vineyard fails to compile with Arrow 12.0.0

The behaviour that you expect to work:

Vineyard implements the new total_bytes_allocated and num_allocations

[kou]

FYI: I don't try this but the following patch may work:

diff --git a/modules/basic/ds/arrow_shim/memory_pool.cc b/modules/basic/ds/arrow_shim/memory_pool.cc
index 9d9601bf..abb2ca86 100644
--- a/modules/basic/ds/arrow_shim/memory_pool.cc
+++ b/modules/basic/ds/arrow_shim/memory_pool.cc
@@ -29,6 +29,8 @@ namespace memory {
 
 VineyardMemoryPool::VineyardMemoryPool(Client& client) : client_(client) {
   bytes_allocated_.store(0);
+  total_bytes_allocated_.store(0);
+  num_allocations_.store(0);
 }
 
 VineyardMemoryPool::~VineyardMemoryPool() {
@@ -50,6 +52,8 @@ arrow::Status VineyardMemoryPool::Allocate(int64_t size, uint8_t** out) {
     {
       std::lock_guard<std::mutex> lock(mutex_);
       bytes_allocated_.fetch_add(size);
+      total_bytes_allocated_.fetch_add(size);
+      num_allocations_.fetch_add(1);
       buffers_.emplace(reinterpret_cast<uintptr_t>(*out), std::move(sbuffer));
     }
     return arrow::Status::OK();
@@ -97,6 +101,11 @@ arrow::Status VineyardMemoryPool::Reallocate(int64_t old_size, int64_t new_size,
   {
     std::lock_guard<std::mutex> lock(mutex_);
     bytes_allocated_.fetch_add(new_size);
+    const auto diff = new_size - old_size;
+    if (diff > 0) {
+      total_bytes_allocated_.fetch_add(diff);
+    }
+    num_allocations_.fetch_add(1);
     buffers_.emplace(reinterpret_cast<uintptr_t>(*ptr), std::move(nsbuffer));
   }
   // remove the original buffer
@@ -171,6 +180,16 @@ int64_t VineyardMemoryPool::bytes_allocated() const {
 /// returns -1
 int64_t VineyardMemoryPool::max_memory() const { return -1; }
 
+/// The number of bytes that were allocated.
+int64_t VineyardMemoryPool::total_bytes_allocated() const {
+  return total_bytes_allocated_.load();
+}
+
+/// The number of allocations or reallocations that were requested.
+int64_t VineyardMemoryPool::num_allocations() const {
+  return num_allocations_.load();
+}
+
 std::string VineyardMemoryPool::backend_name() const { return "vineyard"; }
 
 }  // namespace memory
diff --git a/modules/basic/ds/arrow_shim/memory_pool.h b/modules/basic/ds/arrow_shim/memory_pool.h
index 6f5d56dc..f11d60e2 100644
--- a/modules/basic/ds/arrow_shim/memory_pool.h
+++ b/modules/basic/ds/arrow_shim/memory_pool.h
@@ -75,11 +75,27 @@ class VineyardMemoryPool : public arrow::MemoryPool {
   /// returns -1
   int64_t max_memory() const override;
 
+  /// The number of bytes that were allocated.
+  virtual int64_t total_bytes_allocated() const
+#if defined(ARROW_VERSION) && ARROW_VERSION >= 12000000
+      override
+#endif
+      ;  // NOLINT(whitespace/semicolon)
+
+  /// The number of allocations or reallocations that were requested.
+  virtual int64_t num_allocations() const
+#if defined(ARROW_VERSION) && ARROW_VERSION >= 12000000
+      override
+#endif
+      ;  // NOLINT(whitespace/semicolon)
+
   std::string backend_name() const override;
 
  private:
   Client& client_;
   std::atomic_size_t bytes_allocated_;
+  std::atomic_size_t total_bytes_allocated_;
+  std::atomic_size_t num_allocations_;
   std::mutex mutex_;
   std::map<uintptr_t, std::unique_ptr<BlobWriter>> buffers_;
 };

[sighingnow]

FYI: I don't try this but the following patch may work:

Applied in #1359, thanks!

[kou]

Thanks!