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.

Differential Revision: D56658835
  • Loading branch information
Bikramjeet Vig authored and facebook-github-bot committed Apr 26, 2024
1 parent c2526ba commit 6a9c943
Show file tree
Hide file tree
Showing 3 changed files with 11 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
2 changes: 2 additions & 0 deletions velox/common/memory/tests/MemoryAllocatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,8 @@ 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 6a9c943

Please sign in to comment.