Skip to content

Report actual instead of requested iterations in memory results - #2322

Open
nicholasjng wants to merge 1 commit into
google:mainfrom
nicholasjng:push-oqskqzymwzkw
Open

nicholasjng wants to merge 1 commit into
google:mainfrom
nicholasjng:push-oqskqzymwzkw

Conversation

@nicholasjng

Copy link
Copy Markdown
Contributor

Previously, the run report contained the requested number of memory iterations. When using KeepRunningBatch with a batch size not dividing that number, the memory profiling overshoots, and additional iterations go unreported.

This change sets the number of iterations that were actually run in the memory manager as memory_result.memory_iterations, and adds a regression test exercising the overshooting path via KeepRunningBatch().


Found in experiments with KeepRunningBatch and memory profiling in mew.

disclaimer: I used AI for the creation of the unit test, reviewed it with the existing profiler_manager_iterations_test.cc as reference, liked what I saw.

@nicholasjng

nicholasjng commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor Author

I could make it a gtest instead, and use EXPECT_DOUBLE_EQ for allocs_per_iter there.

EDIT: converted to a gtest. Bazel needs no updates because of the file matching logic.

Previously, the run report contained the requested number of memory iterations.
When using `KeepRunningBatch` with a batch size not dividing that number, the
memory profiling overshoots, and additional iterations go unreported.

This change sets the number of iterations that were actually run in the memory
manager as `memory_result.memory_iterations`, and adds a regression test
exercising the overshooting path via `KeepRunningBatch()`.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant