Skip to content

Cap clock-resolution estimation iterations to avoid OOM - #3196

Closed
22elix3r wants to merge 1 commit into
catchorg:develfrom
22elix3r:fix/3180-clock-resolution-iteration-limit
Closed

Cap clock-resolution estimation iterations to avoid OOM#3196
22elix3r wants to merge 1 commit into
catchorg:develfrom
22elix3r:fix/3180-clock-resolution-iteration-limit

Conversation

@22elix3r

Copy link
Copy Markdown

Summary

Clock-resolution estimation in Catch2's benchmark environment probe is only bounded by a hard-coded 500ms window. On a cheap, high-resolution clock (for example an embedded qemu-system-arm target with 16MB RAM) that window can request well over a million TimePoint samples, exhausting memory inside resolution() and throwing std::bad_alloc.

This change adds a sample cap so warmup() and estimate_clock_resolution() stop doubling once 100000 iterations are reached, even if the time budget has not elapsed.

Related Issue

Fixes #3180

Changes Made

  • Added clock_resolution_estimation_iteration_limit (100000) next to the other environment-probe constants.
  • Extended run_for_at_least with an optional max_iterations argument (default remains the historical 1 << 30 "optimized away" ceiling).
  • Passed the new cap from warmup() and estimate_clock_resolution(), which are the two paths that allocate O(n) TimePoints via resolution().
  • Left the two-phase sampling in resolution() unchanged so consecutive Clock::now() calls are still measured with minimal work between them.
  • Did not add a new CLI flag: the OOM is caused by unbounded sample count, not the time budget, and the environment probe is cached once per process. A hard cap is the safer default for memory-constrained targets.

Testing

  • Added clock resolution estimation is iteration-capped using counting_clock at 1ns/tick, which would otherwise demand hundreds of millions of samples to fill 500ms.
  • Added run_for_at_least respects max_iterations.
  • Extended estimate_clock_resolution to assert samples_seen stays within the cap.
  • Updated the run benchmark fake-clock tick count for the capped probe.
  • Regenerated SelfTest approval baselines for the new cases.
  • ctest in a Debug basic-tests build: 82/82 passed, including RunTests and ApprovalTests.

Notes

  • Adding safeguard to clock resolution estimation (Issue 3180) #3182 also targets this issue. That patch rewrote estimate_clock_resolution and stopped storing every TimePoint, but it did not cap warmup() (which uses the same resolution() allocator), left SelfTest's exact fake-clock assertion failing, and has had no maintainer review. This PR keeps run_for_at_least as the single doubling loop, caps both warmup and resolution collection, and updates the tests.
  • 100000 matches clock_cost_estimation_tick_limit already in the same header. Peak extra allocation is about 1.6MB (TimePoints + deltas) instead of ~20MB at the 1.28 million samples reported in Clock resolution estimation iterations limit #3180.
  • Amalgamated distribution files were not updated, per contributing guidelines.

warmup() and estimate_clock_resolution() previously only stopped after
a time budget, so a cheap high-resolution clock could request over a
million TimePoint samples and exhaust memory on embedded targets.

run_for_at_least now accepts an optional sample cap; clock-resolution
collection uses 100000. Existing callers keep the historical 2^30
optimized-away ceiling.

Fixes catchorg#3180

Signed-off-by: elix3r <157088510+22elix3r@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 27, 2026 02:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.18%. Comparing base (317ac1e) to head (8e2256a).

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #3196      +/-   ##
==========================================
- Coverage   91.21%   91.18%   -0.03%     
==========================================
  Files         206      206              
  Lines        9029     9033       +4     
==========================================
+ Hits         8235     8236       +1     
- Misses        794      797       +3     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@22elix3r

Copy link
Copy Markdown
Author

Ready for review. I cannot request a reviewer from this account.

@22elix3r

Copy link
Copy Markdown
Author

Withdrawing: I missed that this issue already had earlier open PRs. Sorry for the noise — leaving the field to those patches.

@22elix3r

Copy link
Copy Markdown
Author

Withdrawing this PR so the earlier open work on the issue can proceed.

@22elix3r 22elix3r closed this Aug 27, 2026
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.

Clock resolution estimation iterations limit

2 participants