RFR: 7903750: TimeBudget (-tb) does not fulfill its promisses [v5]

[Galder Zamarreño]

On Fri, 21 Mar 2025 12:54:29 GMT, Jiří Vaněk <jvanek at openjdk.org> wrote:

>> jcstress-core/src/main/java/org/openjdk/jcstress/TimeBudget.java line 156:
>> 
>>> 154:         }
>>> 155:         if (expectedPerTest * 2 < budget.milliseconds() * 2l) {
>>> 156:             System.out.println(" + +++ WARNING - your time budget will not be used. Tests will end much sooner.");
>> 
>> See how other warning messages are printed. You should follow similar format e.g.
>> 
>> 
>> System.out.println("WARNING: Allocation profiling is not available: " + e.getMessage());
>
> true. I had adjusted it so it is `+ WARNING: ...` nad same for `FATAL`:
> 
>  + FATAL: - your tests will never finish as expected. They will run much longer 
>  | For 169716 with concurrency factor of 1 You have requested/been given time budget of: 01:00:00
>  | That is ~21 ms per test
>  + +++ However the real time will be converging to: 5d+21:25:48 +++
>  | You can play with internal properties name(value/eta):
>  |   jcstress.timeBudget.defaultPerTestMs(3000ms/5d+21:25:48)
>  |   jcstress.timeBudget.minTimeMs(30ms/01:24:51)
>  |   jcstress.timeBudget.maxTimeMs(60000ms/117d+20:36:00)
>  | Which are setting up the exact times the individual tests are trying to converage to.
>  + Use with caution! Test run below 100ms is moreover jeopardize the purpose. And will not squeeze the time as you wish.
> 
> 
> I had kept the leading `+` so it continue to look aligned and is clea where it starts and ends.  If you wish me to remove also leading `+ ` then a slightly different approach would need to be taken.

Hmmm I see, that might be ok for multi-line comments, but if I were you I would look for other multi-line outputs and see what is done there. In JMH multi line comments are just printed as they are, e.g. https://github.com/openjdk/jmh/blob/master/jmh-core/src/main/java/org/openjdk/jmh/runner/format/TextReportFormat.java#L237

>> jcstress-core/src/main/java/org/openjdk/jcstress/TimeBudget.java line 160:
>> 
>>> 158:         }
>>> 159:         if (print) {
>>> 160:             System.out.println(" | For " + expectedTests + " with concurrency factor of " + getConcurentTestsFactor()
>> 
>> If it was me, I would switch these `println` calls for `printf` to make the messages more readable.
>
> I had rewrote it to printf, but at the end found the println slightly more readable. It seems println is a bit more used then printf in the prject, so unless yuo insists I will keep println.

No, that's fine to leave as is

-------------

PR Review Comment: https://git.openjdk.org/jcstress/pull/161#discussion_r2010681998
PR Review Comment: https://git.openjdk.org/jcstress/pull/161#discussion_r2010683305