Make memory pool dbg message more readable (#10778)

Summary:
Current memory pool dbg message prints every leaked allocation without aggregation. When many allocations are present it becomes very long and unreadable. This PR improves it by aggregating each allocation sites information and print the allocations on size descending order to expose the biggest leaks first.

Pull Request resolved: #10778

Reviewed By: xiaoxmeng

Differential Revision: D61484697

Pulled By: tanjialiang

fbshipit-source-id: 619b981b49a96ec66b8bc8afb75646140fc1c83f

velox/common/memory/MemoryPool.cpp

@@ -1252,11 +1252,34 @@ void MemoryPoolImpl::leakCheckDbg() {
   std::ostream oss(&buf);
   oss << "Detected total of " << debugAllocRecords_.size()
       << " leaked allocations:\n";
+  struct AllocationStats {
+    uint64_t size{0};
+    uint64_t numAllocations{0};
+  };
+  std::unordered_map<std::string, AllocationStats> sizeAggregatedRecords;
   for (const auto& itr : debugAllocRecords_) {
     const auto& allocationRecord = itr.second;
-    oss << "======== Leaked memory allocation of " << allocationRecord.size
-        << " bytes ========\n"
-        << allocationRecord.callStack.toString();
+    const auto stackStr = allocationRecord.callStack.toString();
+    if (sizeAggregatedRecords.count(stackStr) == 0) {
+      sizeAggregatedRecords[stackStr] = AllocationStats();
+    }
+    sizeAggregatedRecords[stackStr].size += allocationRecord.size;
+    ++sizeAggregatedRecords[stackStr].numAllocations;
+  }
+  std::vector<std::pair<std::string, AllocationStats>> sortedRecords(
+      sizeAggregatedRecords.begin(), sizeAggregatedRecords.end());
+  std::sort(
+      sortedRecords.begin(),
+      sortedRecords.end(),
+      [](const std::pair<std::string, AllocationStats>& a,
+         std::pair<std::string, AllocationStats>& b) {
+        return a.second.size > b.second.size;
+      });
+  for (const auto& pair : sortedRecords) {
+    oss << "======== Leaked memory from " << pair.second.numAllocations
+        << " total allocations of " << succinctBytes(pair.second.size)
+        << " total size ========\n"
+        << pair.first << "\n";
   }
   VELOX_FAIL(buf.str());
 }

velox/common/memory/tests/MemoryPoolTest.cpp

@@ -40,9 +40,7 @@ constexpr int64_t KB = 1024L;
 constexpr int64_t MB = 1024L * KB;
 constexpr int64_t GB = 1024L * MB;
 
-namespace facebook {
-namespace velox {
-namespace memory {
+namespace facebook::velox::memory {
 
 struct TestParam {
   bool useMmap;
@@ -98,6 +96,7 @@ class MemoryPoolTest : public testing::TestWithParam<TestParam> {
 
   void setupMemory(
       MemoryManagerOptions options = {
+          .debugEnabled = true,
           .allocatorCapacity = kDefaultCapacity,
           .arbitratorCapacity = kDefaultCapacity,
           .arbitratorReservedCapacity = 1LL << 30}) {
@@ -433,7 +432,6 @@ TEST_P(MemoryPoolTest, DISABLED_memoryLeakCheck) {
   auto child = root->addLeafChild("elastic_quota", isLeafThreadSafe_);
   const int64_t kChunkSize{32L * MB};
   void* oneChunk = child->allocate(kChunkSize);
-  FLAGS_velox_memory_leak_check_enabled = true;
   ASSERT_DEATH(child.reset(), "");
   child->free(oneChunk, kChunkSize);
 }
@@ -3884,6 +3882,4 @@ VELOX_INSTANTIATE_TEST_SUITE_P(
     MemoryPoolTest,
     testing::ValuesIn(MemoryPoolTest::getTestParams()));
 
-} // namespace memory
-} // namespace velox
-} // namespace facebook
+} // namespace facebook::velox::memory