[GitHub] [arrow] wesm commented on a change in pull request #8533: ARROW-10080: [R] Call gc() and try again in MemoryPool

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

wesm commented on a change in pull request #8533:
URL: https://github.com/apache/arrow/pull/8533#discussion_r512231253



##########
File path: r/src/memorypool.cpp
##########
@@ -19,10 +19,45 @@
 #if defined(ARROW_R_WITH_ARROW)
 #include <arrow/memory_pool.h>
 
+class GcMemoryPool : public arrow::MemoryPool {
+ public:
+  GcMemoryPool()
+      : pool_(arrow::default_memory_pool()), gc_(cpp11::package("base")["gc"]) {}
+
+  arrow::Status Allocate(int64_t size, uint8_t** out) override {
+    return GcAndTryAgain([&] { return pool_->Allocate(size, out); });
+  }
+
+  arrow::Status Reallocate(int64_t old_size, int64_t new_size, uint8_t** ptr) override {
+    return GcAndTryAgain([&] { return pool_->Reallocate(old_size, new_size, ptr); });
+  }
+
+  void Free(uint8_t* buffer, int64_t size) override { pool_->Free(buffer, size); }
+
+  int64_t bytes_allocated() const override { return pool_->bytes_allocated(); }
+
+  int64_t max_memory() const override { return pool_->max_memory(); }
+
+  std::string backend_name() const override { return pool_->backend_name() + "-gc"; }
+
+ private:
+  template <typename Call>
+  arrow::Status GcAndTryAgain(const Call& call) {
+    if (call().ok()) {

Review comment:
       Can you add a comment explaining the circumstances when this could fail / why (also put this in the PR description)?

##########
File path: r/src/memorypool.cpp
##########
@@ -19,10 +19,45 @@
 #if defined(ARROW_R_WITH_ARROW)
 #include <arrow/memory_pool.h>
 
+class GcMemoryPool : public arrow::MemoryPool {
+ public:
+  GcMemoryPool()
+      : pool_(arrow::default_memory_pool()), gc_(cpp11::package("base")["gc"]) {}
+
+  arrow::Status Allocate(int64_t size, uint8_t** out) override {
+    return GcAndTryAgain([&] { return pool_->Allocate(size, out); });
+  }
+
+  arrow::Status Reallocate(int64_t old_size, int64_t new_size, uint8_t** ptr) override {
+    return GcAndTryAgain([&] { return pool_->Reallocate(old_size, new_size, ptr); });
+  }
+
+  void Free(uint8_t* buffer, int64_t size) override { pool_->Free(buffer, size); }
+
+  int64_t bytes_allocated() const override { return pool_->bytes_allocated(); }
+
+  int64_t max_memory() const override { return pool_->max_memory(); }
+
+  std::string backend_name() const override { return pool_->backend_name() + "-gc"; }
+
+ private:
+  template <typename Call>
+  arrow::Status GcAndTryAgain(const Call& call) {
+    if (call().ok()) {
+      return arrow::Status::OK();
+    }
+    // ARROW-10080
+    gc_();

Review comment:
       Are `cpp11::function` objects threadsafe?




----------------------------------------------------------------
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