marin-ma commented on code in PR #11493:
URL: 
https://github.com/apache/incubator-gluten/pull/11493#discussion_r2733238789


##########
cpp/velox/memory/VeloxMemoryManager.cc:
##########
@@ -327,22 +330,17 @@ int64_t 
shrinkVeloxMemoryPool(velox::memory::MemoryManager* mm, velox::memory::M
 std::shared_ptr<arrow::MemoryPool> 
VeloxMemoryManager::getOrCreateArrowMemoryPool(const std::string& name) {
   std::lock_guard<std::mutex> l(mutex_);
   if (const auto it = arrowPools_.find(name); it != arrowPools_.end()) {
-    auto pool = it->second.lock();
-    VELOX_CHECK_NOT_NULL(pool, "Arrow memory pool {} has been destructed", 
name);
-    return pool;
+    if (auto pool = it->second.lock()) {
+      return pool;
+    }
+    arrowPools_.erase(name);
   }
-  auto pool = std::make_shared<ArrowMemoryPool>(
-      blockListener_.get(), [this, name](arrow::MemoryPool* pool) { 
this->dropMemoryPool(name); });
+
+  auto pool = std::make_shared<ArrowMemoryPool>(blockListener_.get());

Review Comment:
   Using `weak_ptr` can help to clarify the ownership of the arrow pool. For 
example if the arrow pool is used by shuffle A, it should be destroyed together 
with the shuffle writer A itself. And then the next shuffle B will create a new 
pool. In this way we can also track the memory allocation status for each 
shuffle separately.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to