Skip to content

Commit

Permalink
Add option to print out the child memory pools with empty usage in tr…
Browse files Browse the repository at this point in the history 
…eeMemoryUsage (#8312)

Summary:
Add a detail option print out the child memory pools with empty memory usage
in treeMemoryUsage. And use it in memory pool leak error message

Pull Request resolved: #8312

Reviewed By: tanjialiang

Differential Revision: D52639456

Pulled By: xiaoxmeng

fbshipit-source-id: 6031ddf9741680444cc0a2b4da2e0d9e55a4dbc3
  • Loading branch information
@xiaoxmeng @facebook-github-bot
xiaoxmeng authored and facebook-github-bot committed Jan 10, 2024
1 parent f15e096 commit e8892f6
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 41 deletions.
4 changes: 2 additions & 2 deletions velox/common/memory/Memory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,14 +280,14 @@ std::string MemoryManager::toString(bool detail) const {
<< "\n";
out << "List of root pools:\n";
if (detail) {
out << defaultRoot_->treeMemoryUsage();
out << defaultRoot_->treeMemoryUsage(false);
} else {
out << "\t" << defaultRoot_->name() << "\n";
}
std::vector<std::shared_ptr<MemoryPool>> pools = getAlivePools();
for (const auto& pool : pools) {
if (detail) {
out << pool->treeMemoryUsage();
out << pool->treeMemoryUsage(true);
} else {
out << "\t" << pool->name() << "\n";
}
Expand Down
24 changes: 14 additions & 10 deletions velox/common/memory/MemoryPool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,11 @@ void treeMemoryUsageVisitor(
MemoryPool* pool,
size_t indent,
MemoryUsageHeap& topLeafMemUsages,
bool skipEmptyPool,
std::stringstream& out) {
const MemoryPool::Stats stats = pool->stats();
// Avoid logging empty pools.
if (stats.empty()) {
// Avoid logging empty pools if 'skipEmptyPool' is true.
if (stats.empty() && skipEmptyPool) {
return;
}
const MemoryUsage usage{
Expand All @@ -127,18 +128,21 @@ void treeMemoryUsageVisitor(
out << std::string(indent, ' ') << usage.toString() << "\n";

if (pool->kind() == MemoryPool::Kind::kLeaf) {
if (stats.empty()) {
return;
}
static const size_t kTopNLeafMessages = 10;
topLeafMemUsages.push(usage);
if (topLeafMemUsages.size() > kTopNLeafMessages) {
topLeafMemUsages.pop();
}
return;
}
pool->visitChildren(
[&, indent = indent + kCapMessageIndentSize](MemoryPool* pool) {
treeMemoryUsageVisitor(pool, indent, topLeafMemUsages, out);
return true;
});
pool->visitChildren([&, indent = indent + kCapMessageIndentSize](
MemoryPool* pool) {
treeMemoryUsageVisitor(pool, indent, topLeafMemUsages, skipEmptyPool, out);
return true;
});
}

std::string capacityToString(int64_t capacity) {
Expand Down Expand Up @@ -884,9 +888,9 @@ void MemoryPoolImpl::decrementReservation(uint64_t size) noexcept {
sanityCheckLocked();
}

std::string MemoryPoolImpl::treeMemoryUsage() const {
std::string MemoryPoolImpl::treeMemoryUsage(bool skipEmptyPool) const {
if (parent_ != nullptr) {
return parent_->treeMemoryUsage();
return parent_->treeMemoryUsage(skipEmptyPool);
}
if (FLAGS_velox_suppress_memory_capacity_exceeding_error_message) {
return "";
Expand All @@ -905,7 +909,7 @@ std::string MemoryPoolImpl::treeMemoryUsage() const {

MemoryUsageHeap topLeafMemUsages;
visitChildren([&, indent = kCapMessageIndentSize](MemoryPool* pool) {
treeMemoryUsageVisitor(pool, indent, topLeafMemUsages, out);
treeMemoryUsageVisitor(pool, indent, topLeafMemUsages, skipEmptyPool, out);
return true;
});

Expand Down
53 changes: 27 additions & 26 deletions velox/common/memory/MemoryPool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,9 @@ class MemoryPool : public std::enable_shared_from_this<MemoryPool> {
virtual std::string toString() const = 0;

/// Invoked to generate a descriptive memory usage summary of the entire tree.
/// MemoryPoolImpl::treeMemoryUsage()
virtual std::string treeMemoryUsage() const = 0;
/// MemoryPoolImpl::treeMemoryUsage(). If 'skipEmptyPool' is true, then skip
/// print out the child memory pools with empty memory usage.
virtual std::string treeMemoryUsage(bool skipEmptyPool = true) const = 0;

/// Indicates if this is a leaf memory pool or not.
FOLLY_ALWAYS_INLINE bool isLeaf() const {
Expand Down Expand Up @@ -650,30 +651,30 @@ class MemoryPoolImpl : public MemoryPool {
return toStringLocked();
}

// Detailed debug pool state printout by traversing the pool structure from
// the root memory pool.
//
// Exceeded memory cap of 5.00MB when requesting 2.00MB
// default_root_1 usage 5.00MB peak 5.00MB
// task.test_cursor 1 usage 5.00MB peak 5.00MB
// node.N/A usage 0B peak 0B
// op.N/A.0.0.CallbackSink usage 0B peak 0B
// node.2 usage 4.00MB peak 4.00MB
// op.2.0.0.Aggregation usage 3.77MB peak 3.77MB
// node.1 usage 1.00MB peak 1.00MB
// op.1.0.0.FilterProject usage 12.00KB peak 12.00KB
// node.3 usage 0B peak 0B
// op.3.0.0.OrderBy usage 0B peak 0B
// node.0 usage 0B peak 0B
// op.0.0.0.Values usage 0B peak 0B
//
// Top 5 leaf memory pool usages:
// op.2.0.0.Aggregation usage 3.77MB peak 3.77MB
// op.1.0.0.FilterProject usage 12.00KB peak 12.00KB
// op.N/A.0.0.CallbackSink usage 0B peak 0B
// op.3.0.0.OrderBy usage 0B peak 0B
// op.0.0.0.Values usage 0B peak 0B
std::string treeMemoryUsage() const override;
/// Detailed debug pool state printout by traversing the pool structure from
/// the root memory pool.
///
/// Exceeded memory cap of 5.00MB when requesting 2.00MB
/// default_root_1 usage 5.00MB peak 5.00MB
/// task.test_cursor 1 usage 5.00MB peak 5.00MB
/// node.N/A usage 0B peak 0B
/// op.N/A.0.0.CallbackSink usage 0B peak 0B
/// node.2 usage 4.00MB peak 4.00MB
/// op.2.0.0.Aggregation usage 3.77MB peak 3.77MB
/// node.1 usage 1.00MB peak 1.00MB
/// op.1.0.0.FilterProject usage 12.00KB peak 12.00KB
/// node.3 usage 0B peak 0B
/// op.3.0.0.OrderBy usage 0B peak 0B
/// node.0 usage 0B peak 0B
/// op.0.0.0.Values usage 0B peak 0B
///
/// Top 5 leaf memory pool usages:
/// op.2.0.0.Aggregation usage 3.77MB peak 3.77MB
/// op.1.0.0.FilterProject usage 12.00KB peak 12.00KB
/// op.N/A.0.0.CallbackSink usage 0B peak 0B
/// op.3.0.0.OrderBy usage 0B peak 0B
/// op.0.0.0.Values usage 0B peak 0B
std::string treeMemoryUsage(bool skipEmptyPool = true) const override;

Stats stats() const override;

Expand Down
15 changes: 15 additions & 0 deletions velox/common/memory/tests/MemoryManagerTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <gtest/gtest.h>

#include <gmock/gmock-matchers.h>
#include "velox/common/base/VeloxException.h"
#include "velox/common/base/tests/GTestUtils.h"
#include "velox/common/memory/MallocAllocator.h"
Expand Down Expand Up @@ -280,6 +281,20 @@ TEST_F(MemoryManagerTest, defaultMemoryManager) {
ASSERT_EQ(
managerB.toString(),
"Memory Manager[capacity UNLIMITED alignment 64B usedBytes 0B number of pools 1\nList of root pools:\n\t__sys_root__\nMemory Allocator[MALLOC capacity UNLIMITED allocated bytes 0 allocated pages 0 mapped pages 0]\nARBIRTATOR[NOOP CAPACITY[UNLIMITED]]]");
const std::string detailedManagerStr = managerA.toString(true);
ASSERT_THAT(
detailedManagerStr,
testing::HasSubstr(
"Memory Manager[capacity UNLIMITED alignment 64B usedBytes 0B number of pools 1\nList of root pools:\n__sys_root__ usage 0B reserved 0B peak 0B\n"));
ASSERT_THAT(
detailedManagerStr,
testing::HasSubstr("__sys_spilling__ usage 0B reserved 0B peak 0B\n"));
for (int i = 0; i < 32; ++i) {
ASSERT_THAT(
managerA.toString(true),
testing::HasSubstr(fmt::format(
"default_shared_leaf_pool_{} usage 0B reserved 0B peak 0B\n", i)));
}
}

// TODO: remove this test when remove deprecatedAddDefaultLeafMemoryPool.
Expand Down
16 changes: 14 additions & 2 deletions velox/common/memory/tests/MemoryPoolTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class MemoryPoolTest : public testing::TestWithParam<TestParam> {
MemoryReclaimer::Stats stats_;
};

TEST_P(MemoryPoolTest, Ctor) {
TEST_P(MemoryPoolTest, ctor) {
constexpr uint16_t kAlignment = 64;
setupMemory({.alignment = 64, .allocatorCapacity = kDefaultCapacity});
MemoryManager& manager = *getMemoryManager();
Expand Down Expand Up @@ -203,7 +203,7 @@ TEST_P(MemoryPoolTest, Ctor) {
"Memory pool rootWithZeroMaxCapacity max capacity can't be zero");
}

TEST_P(MemoryPoolTest, AddChild) {
TEST_P(MemoryPoolTest, addChild) {
MemoryManager& manager = *getMemoryManager();
auto root = manager.addRootPool("root");
ASSERT_EQ(root->parent(), nullptr);
Expand All @@ -227,6 +227,18 @@ TEST_P(MemoryPoolTest, AddChild) {
ASSERT_EQ(root->getChildCount(), 1);
childOne = root->addLeafChild("child_one", isLeafThreadSafe_);
ASSERT_EQ(root->getChildCount(), 2);
ASSERT_EQ(root->treeMemoryUsage(), "root usage 0B reserved 0B peak 0B\n");
ASSERT_EQ(root->treeMemoryUsage(true), "root usage 0B reserved 0B peak 0B\n");
const std::string treeUsageWithEmptyPool = root->treeMemoryUsage(false);
ASSERT_THAT(
treeUsageWithEmptyPool,
testing::HasSubstr("root usage 0B reserved 0B peak 0B\n"));
ASSERT_THAT(
treeUsageWithEmptyPool,
testing::HasSubstr("child_one usage 0B reserved 0B peak 0B\n"));
ASSERT_THAT(
treeUsageWithEmptyPool,
testing::HasSubstr("child_two usage 0B reserved 0B peak 0B\n"));
}

TEST_P(MemoryPoolTest, dropChild) {
Expand Down
2 changes: 1 addition & 1 deletion velox/dwio/dwrf/test/WriterFlushTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ class MockMemoryPool : public velox::memory::MemoryPool {
VELOX_UNSUPPORTED("{} unsupported", __FUNCTION__);
}

std::string treeMemoryUsage() const override {
std::string treeMemoryUsage(bool /*unused*/) const override {
VELOX_UNSUPPORTED("{} unsupported", __FUNCTION__);
}

Expand Down

0 comments on commit e8892f6

Please sign in to comment.