google / google/benchmark

[BUG] Memory manager counts allocations not part of benchmark

Open
#2,149 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
10.4k
Forks
1.8k
Avg merge
2d 4h
Merged PRs (30d)
8

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`:

```c++
MemoryManager::Result BenchmarkRunner::RunMemoryManager(
IterationCount memory_iterations) {
memory_manager->Start();
std::unique_ptr 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):

```c++
MemoryManager::Result BenchmarkRunner::RunMemoryManager(
IterationCount memory_iterations) {
std::unique_ptr 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:
1. Implement `MemoryManager` that hooks `malloc` and friends
2. Reset your `MemoryManager`'s `num_allocs` in `Start()`
3. Report your `MemoryManager`'s `num_allocs` in `Stop()`
4. Create a non-allocating benchmark (e.g. do nothing or fill an `std::array`)
5. Run benchmark with `--benchmark_format=json`
6. Notice how `"allocs_per_iter"` in the JSON is not zero

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.