[BUG] Memory manager counts allocations not part of benchmark
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 68/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Quiet
- Tech stack
- cpp
- Domain
- performance
Research direction
Start in src/benchmark_runner.cc at BenchmarkRunner::RunMemoryManager and compare the resource setup and teardown with the MemoryManager Start()/Stop() calls. Reproduce with a non-allocating benchmark and a MemoryManager that reports allocs_per_iter in JSON. Done means setup allocations are excluded from the measured interval and teardown allocations occur after Stop().
Written by the indexing model from the issue text.
Description
Describe the bug
When using a MemoryManager, the Start() function is called before some more "internal" objects are created. This makes it impossible to get e.g. a 100% accurate allocation count.
When the call to the MemoryManager's Start() is made, other objects are created/allocated before the test is started. Here is e.g. RunMemoryManager from src/benchmark_runner.cc:
MemoryManager::Result BenchmarkRunner::RunMemoryManager(
IterationCount memory_iterations) {
memory_manager->Start();
std::unique_ptr<internal::ThreadManager> manager;
manager.reset(new internal::ThreadManager(1));
b.Setup();
RunInThread(&b, memory_iterations, 0, manager.get(),
perf_counters_measurement_ptr,
/*profiler_manager=*/nullptr);
manager.reset();
b.Teardown();
MemoryManager::Result memory_result;
memory_manager->Stop(memory_result);
memory_result.memory_iterations = memory_iterations;
return memory_result;
}
As is clear from the code, after calling MemoryManager::Start(), a new internal::ThreadManager object is created on the heap, and b.Setup() is called. Then, before MemoryManager::Stop() is called, manager.reset() and b.TearDown() are called.
IMHO both the internal::ThreadManager creation and the b.Setup()/b.Teardown() should happen before, respectively after, calling Start()/Stop(). That way only the memory use inside the for (auto _ : state) { ... } loop will be tracked, which is what I assume most people would be interested in.
I.e. I would have thought RunMemoryManager would have been implemented like this (note that memory_result is also moved in front of Start(), although it currently doesn't allocate):
MemoryManager::Result BenchmarkRunner::RunMemoryManager(
IterationCount memory_iterations) {
std::unique_ptr<internal::ThreadManager> manager;
manager.reset(new internal::ThreadManager(1));
b.Setup();
MemoryManager::Result memory_result;
memory_manager->Start();
RunInThread(&b, memory_iterations, 0, manager.get(),
perf_counters_measurement_ptr,
/*profiler_manager=*/nullptr);
memory_manager->Stop(memory_result);
manager.reset();
b.Teardown();
memory_result.memory_iterations = memory_iterations;
return memory_result;
}
System
Which OS, compiler, and compiler version are you using:
- OS: Ubuntu 24.04 LTS
- Compiler and version: GCC 12
To reproduce
Steps to reproduce the behavior:
- Implement
MemoryManagerthat hooksmallocand friends - Reset your
MemoryManager'snum_allocsinStart() - Report your
MemoryManager'snum_allocsinStop() - Create a non-allocating benchmark (e.g. do nothing or fill an
std::array) - Run benchmark with
--benchmark_format=json - Notice how
"allocs_per_iter"in the JSON is not zero
- Dominant language
- C++
- Stars
- 10.4k
- Forks
- 1.8k
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 8
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from google/benchmark
-
Difficulty 4/5 3-5 days Newbie friendliness 64/100
-
Difficulty 3/5 1-2 days Newbie friendliness 68/100
-
Difficulty 5/5 Over a week Newbie friendliness 42/100
-
Difficulty 4/5 3-5 days Newbie friendliness 48/100
-
Difficulty 3/5 1-2 days Newbie friendliness 58/100
All issues in google/benchmark
Similar issues
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
AXERA-TECH/ax-llm#77 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
-
bug-unconfirmed
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
NVIDIA/cuda-samples#453 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
infiniflow/infinity#3502 ·