Skip to content

Commit

Permalink
Disable allocator stats collection by default
Browse files Browse the repository at this point in the history
Summary:
Allocator stats collection was designed to be a debugging tool and
can be expensive as allocation happens on the hotpath and stats
collection can add a non-trivial amount to it. Therefore we are
disabling it by default.

Additionally this change improves the toString() API for the struct
that accounts for these stats but adding the number of allocations
to it.

Reviewed By: xiaoxmeng

Differential Revision: D56658835

fbshipit-source-id: 3cf55087e441000e7acce179f911afd37965ca2f
  • Loading branch information
Bikramjeet Vig authored and facebook-github-bot committed Apr 30, 2024
1 parent 85fd5c6 commit d86b1f2
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 5 deletions.
12 changes: 8 additions & 4 deletions velox/common/memory/MemoryAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,15 +364,18 @@ std::string Stats::toString() const {
std::stringstream out;
int64_t totalClocks = 0;
int64_t totalBytes = 0;
int64_t totalAllocations = 0;
for (auto i = 0; i < sizes.size(); ++i) {
totalClocks += sizes[i].clocks();
totalBytes += sizes[i].totalBytes;
totalAllocations += sizes[i].numAllocations;
}
out << fmt::format(
"Alloc: {}MB {} Gigaclocks, {}MB advised\n",
"Alloc: {}MB {} Gigaclocks {} Allocations, {}MB advised\n",
totalBytes >> 20,
totalClocks >> 30,
numAdvise >> 8);
numAdvise >> 8,
totalAllocations);

// Sort the size classes by decreasing clocks.
std::vector<int32_t> indices(sizes.size());
Expand All @@ -386,10 +389,11 @@ std::string Stats::toString() const {
break;
}
out << fmt::format(
"Size {}K: {}MB {} Megaclocks\n",
"Size {}K: {}MB {} Megaclocks {} Allocations\n",
sizes[i].size * 4,
sizes[i].totalBytes >> 20,
sizes[i].clocks() >> 20);
sizes[i].clocks() >> 20,
sizes[i].numAllocations);
}
return out.str();
}
Expand Down
32 changes: 32 additions & 0 deletions velox/common/memory/tests/MemoryAllocatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,10 +632,42 @@ TEST_P(MemoryAllocatorTest, allocationClass2) {
allocation->clear();
}

TEST_P(MemoryAllocatorTest, stats) {
const std::vector<MachinePageCount>& sizes = instance_->sizeClasses();
MachinePageCount capacity = kCapacityPages;
for (auto i = 0; i < sizes.size(); ++i) {
std::unique_ptr<Allocation> allocation = std::make_unique<Allocation>();
auto size = sizes[i];
ASSERT_TRUE(allocate(size, *allocation));
ASSERT_GT(instance_->numAllocated(), 0);
instance_->freeNonContiguous(*allocation);
auto stats = instance_->stats();
ASSERT_EQ(0, stats.sizes[i].clocks());
ASSERT_EQ(stats.sizes[i].totalBytes, 0);
ASSERT_EQ(stats.sizes[i].numAllocations, 0);
}

gflags::FlagSaver flagSaver;
FLAGS_velox_time_allocations = true;
for (auto i = 0; i < sizes.size(); ++i) {
std::unique_ptr<Allocation> allocation = std::make_unique<Allocation>();
auto size = sizes[i];
ASSERT_TRUE(allocate(size, *allocation));
ASSERT_GT(instance_->numAllocated(), 0);
instance_->freeNonContiguous(*allocation);
auto stats = instance_->stats();
ASSERT_LT(0, stats.sizes[i].clocks());
ASSERT_GE(stats.sizes[i].totalBytes, size * AllocationTraits::kPageSize);
ASSERT_GE(stats.sizes[i].numAllocations, 1);
}
}

TEST_P(MemoryAllocatorTest, singleAllocation) {
if (!useMmap_ && enableReservation_) {
return;
}
gflags::FlagSaver flagSaver;
FLAGS_velox_time_allocations = true;
const std::vector<MachinePageCount>& sizes = instance_->sizeClasses();
MachinePageCount capacity = kCapacityPages;
for (auto i = 0; i < sizes.size(); ++i) {
Expand Down
2 changes: 1 addition & 1 deletion velox/flag_definitions/flags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ DEFINE_int32(

DEFINE_bool(
velox_time_allocations,
true,
false,
"Record time and volume for large allocation/free");

// Used in common/base/VeloxException.cpp
Expand Down

0 comments on commit d86b1f2

Please sign in to comment.