-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make warmup_time be configurable #1844
Conversation
@@ -213,6 +213,9 @@ namespace Catch { | |||
| Opt( config.benchmarkNoAnalysis ) | |||
["--benchmark-no-analysis"] | |||
( "perform only measurements; do not perform any analysis" ) | |||
| Opt( config.benchmarkWarmupTime, "benchmarkWarmupTime" ) | |||
["--benchmark-warmup-time"] | |||
( "amount of time in milliseconds spent on warming up each test (default: 500)" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not track with the default in code.
<pre>--benchmark-warmup-time</pre> | ||
|
||
Configure the amount of time spent warming up each test. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should get an "introduced in" placeholder.
I'll have proper look later this week, but two things jumped out to me. |
Codecov Report
@@ Coverage Diff @@
## master #1844 +/- ##
==========================================
- Coverage 86.27% 86.23% -0.03%
==========================================
Files 131 131
Lines 3888 3893 +5
==========================================
+ Hits 3354 3357 +3
- Misses 534 536 +2 |
Great! Also, the constant |
IIRC those are done only once, so they shouldn't cause a problem for users with a large numbers of benchmarks. Anyway, I fixed a missing ToC entry in docs and rebased the approvals, I'll merge it after CI passes. |
thanks so much! ❤️ |
There's a constant,
warmup_time
, that is used for both timer statistics discovery, as well as test warmup. The timer statistics usage is fine, but, the default constant of 100ms used for each and every test warmup makes running a large number of benchmarks take an unreasonable amount of time (the project I'm working on has many very small/fast benchmarks).This PR makes that constant be configurable with a command line switch, instead of a
const
.Thanks! ✨