RFR: CODETOOLS-7903748 - jcstress: Test list should honor concurrency settings [v4]

PM duke at openjdk.org
Thu Jul 4 07:58:30 UTC 2024


On Thu, 4 Jul 2024 06:05:51 GMT, Jiří Vaněk <jvanek at openjdk.org> wrote:

>> jcstress-core/src/main/java/org/openjdk/jcstress/infra/runners/TestConfig.java line 264:
>> 
>>> 262:                 ", strideCount: " + strideCount +
>>> 263:                 ", cpuMap: " + cpuMap +
>>> 264:                 ", " + jvmArgs + "}";
>> 
>> kindly modify this to return a valid json, perhaps try running this method locally once in your machine, and copy paste that json in any online json formatter to check if you encounter any errors
>
> I never had an intention to return valid json.  It is used only in verbose output, to differentiate individual variants of tests.  Wihtout the test id, it do no have sense, and the extended info in {} is... complicated. See eg:
> 
> org.openjdk.jcstress.samples.api.API_01_Simple {[actor1, actor2], spinLoopStyle: Thread.onSpinWait(), threads: 2, forkId: 0, maxFootprintMB: 64, compileMode: 0, shClass: (PG 0, CG 0), (PG 0, CG 0), strideSize: 256, strideCount: 40, cpuMap: null, [-XX:+UseBiasedLocking]}
> org.openjdk.jcstress.samples.api.API_01_Simple {[actor1, actor2], spinLoopStyle: Thread.onSpinWait(), threads: 2, forkId: 0, maxFootprintMB: 64, compileMode: 0, shClass: (PG 0, CG 0), (PG 0, CG 0), strideSize: 256, strideCount: 40, cpuMap: null, [-XX:-UseBiasedLocking]}
> 
> 
> 
> To highlight it is not json, was the original usage of `=`,  but I did not had super strong opinion.
> As the output should remain mainly human readable, I'm quite against full json (thus bracket before name, and id before actorNames and jvmArgs).  But on contrary,  the list is indeed huge.
> 
> All matching tests combinations - 3030696
> x
> All matching tests - 4483
> 
> with most of the combinations.
> 
> The main motivation is the line of ""All matching tests combinations - " + testsToPrint.size()" thus real number of run tests. The listing itself is (very) useful candy on top:
> 
> java -jar tests-all/target/jcstress.jar -v -l  -t org.openjdk.jcstress.samples.api.API_01_Simple 
> ...
> All matching tests combinations - 96
> ...
> 
> x
> 
> java -jar tests-all/target/jcstress.jar -l  -t org.openjdk.jcstress.samples.api.API_01_Simple 
> ...
> All matching tests - 1
> ...
> 
> 
> 
> So back to verbose listing output format - I intentionally kept it semi parse-able - grep/sed  on output, and readable enough: name  info in {} , all fields with key id, and obvious arrays in [] without key.
> **AFAICJ, the full json will lower the human readability.** 
> 
> What about this - with "-v" it would print it as it is - human readable, fully distinguishible.  on "-vv or -vvv"  it would print complete fully qulified json. WDYT?

ok sure, gotcha, the human readable output with `-v` option what you are printing right now should be fine, just one small change I would request, instead of doing string concatenation with the `+` operator, can we use append  operation with String Builder or String Buffer whichever is more appropriate here

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

PR Review Comment: https://git.openjdk.org/jcstress/pull/149#discussion_r1665294060


More information about the jcstress-dev mailing list