Cap clock-resolution estimation iterations to avoid OOM - #3196
Closed
22elix3r wants to merge 1 commit into
Closed
Conversation
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>
Codecov Report✅ All modified and coverable lines are covered by tests. 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:
|
Author
|
Ready for review. I cannot request a reviewer from this account. |
Author
|
Withdrawing: I missed that this issue already had earlier open PRs. Sorry for the noise — leaving the field to those patches. |
Author
|
Withdrawing this PR so the earlier open work on the issue can proceed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-armtarget with 16MB RAM) that window can request well over a millionTimePointsamples, exhausting memory insideresolution()and throwingstd::bad_alloc.This change adds a sample cap so
warmup()andestimate_clock_resolution()stop doubling once 100000 iterations are reached, even if the time budget has not elapsed.Related Issue
Fixes #3180
Changes Made
clock_resolution_estimation_iteration_limit(100000) next to the other environment-probe constants.run_for_at_leastwith an optionalmax_iterationsargument (default remains the historical1 << 30"optimized away" ceiling).warmup()andestimate_clock_resolution(), which are the two paths that allocateO(n)TimePoints viaresolution().resolution()unchanged so consecutiveClock::now()calls are still measured with minimal work between them.Testing
clock resolution estimation is iteration-cappedusingcounting_clockat 1ns/tick, which would otherwise demand hundreds of millions of samples to fill 500ms.run_for_at_least respects max_iterations.estimate_clock_resolutionto assertsamples_seenstays within the cap.run benchmarkfake-clock tick count for the capped probe.ctestin a Debugbasic-testsbuild: 82/82 passed, includingRunTestsandApprovalTests.Notes
estimate_clock_resolutionand stopped storing everyTimePoint, but it did not capwarmup()(which uses the sameresolution()allocator), left SelfTest's exact fake-clock assertion failing, and has had no maintainer review. This PR keepsrun_for_at_leastas the single doubling loop, caps both warmup and resolution collection, and updates the tests.clock_cost_estimation_tick_limitalready 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.