jcstress-dev Digest, Vol 71, Issue 6
Prakhar Makhija
matcdac at gmail.com
Thu Jul 4 08:03:57 UTC 2024
Hello Team,
Provided Review Comments for both *openjdk/jcstress PRs : #119 *and* #149*
https://github.com/openjdk/jcstress/pull/119
https://github.com/openjdk/jcstress/pull/149
Approved *openjdk/jcstress PR : #150*
https://github.com/openjdk/jcstress/pull/150
Regards,
*Prakhar Makhija*
+91 78294 00970
matcdac at gmail.com
https://www.linkedin.com/in/prakhar-makhija-17794b107/
On Thu, Jul 4, 2024 at 11:55 AM <jcstress-dev-request at openjdk.org> wrote:
> Send jcstress-dev mailing list submissions to
> jcstress-dev at openjdk.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> https://mail.openjdk.org/mailman/listinfo/jcstress-dev
> or, via email, send a message with subject or body 'help' to
> jcstress-dev-request at openjdk.org
>
> You can reach the person managing the list at
> jcstress-dev-owner at openjdk.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of jcstress-dev digest..."
>
>
> Today's Topics:
>
> 1. Re: RFR: CODETOOLS-7903748 - jcstress: Test list should honor
> concurrency settings [v5] (Ji?? Van?k)
> 2. Re: RFR: CODETOOLS-7903748 - jcstress: Test list should honor
> concurrency settings [v4] (Ji?? Van?k)
> 3. Re: RFR: CODETOOLS-7903748 - jcstress: Test list should honor
> concurrency settings [v4] (Ji?? Van?k)
> 4. Re: RFR: CODETOOLS-7903756 - jcstress shuld not pass
> debugging parameters to subproceses [v4] (Ji?? Van?k)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Thu, 4 Jul 2024 05:49:57 GMT
> From: Ji?? Van?k <jvanek at openjdk.org>
> To: <jcstress-dev at openjdk.org>
> Subject: Re: RFR: CODETOOLS-7903748 - jcstress: Test list should honor
> concurrency settings [v5]
> Message-ID:
> <wRLNZ4GbD_bgjolLGHRy7ehEb_TWfi725EVxWLV1vlM=.
> d6e1c38f-2687-4e1e-af67-500bc6271cb0 at github.com>
>
> Content-Type: text/plain; charset=utf-8
>
> > This is extracting List<TestConfig> configs =prepareRunProgram(classes,
> tests);
> > with all he HW/switches setup to separated method and reusing it in `-l`
> mode
> >
> > I'm aware of triplicated removal of -agentlib, and will clean it up as
> > CODETOOLS-7903756 will progress.
> >
> > -l now honours also verbose mode, in which it prints not just matching
> > tests but all really run tests, and thus enabling much more easy
> > determining of all tests
> >
> > help adjusted.
> >
> > Maybe I'm missing plain quick initial all tests metod now, but with
> > artificial -c MAX it seems doing exactly that
>
> Ji?? Van?k has updated the pull request incrementally with one additional
> commit since the last revision:
>
> LIST_OPTION_DESCRIPTION split to two lines
>
> -------------
>
> Changes:
> - all: https://git.openjdk.org/jcstress/pull/149/files
> - new:
> https://git.openjdk.org/jcstress/pull/149/files/f4ac4222..713c5a2d
>
> Webrevs:
> - full: https://webrevs.openjdk.org/?repo=jcstress&pr=149&range=04
> - incr: https://webrevs.openjdk.org/?repo=jcstress&pr=149&range=03-04
>
> Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
> Patch: https://git.openjdk.org/jcstress/pull/149.diff
> Fetch: git fetch https://git.openjdk.org/jcstress.git
> pull/149/head:pull/149
>
> PR: https://git.openjdk.org/jcstress/pull/149
>
>
> ------------------------------
>
> Message: 2
> Date: Thu, 4 Jul 2024 05:49:57 GMT
> From: Ji?? Van?k <jvanek at openjdk.org>
> To: <jcstress-dev at openjdk.org>
> Subject: Re: RFR: CODETOOLS-7903748 - jcstress: Test list should honor
> concurrency settings [v4]
> Message-ID:
> <c2X65bWtSWiCPXcigFgMCbNt9Dnl3JeOxDHzNBx5udY=.
> d49b7c4e-e620-4c6e-a72b-d31763be8260 at github.com>
>
> Content-Type: text/plain; charset=utf-8
>
> On Wed, 3 Jul 2024 19:30:06 GMT, PM <duke at openjdk.org> wrote:
>
> >> Ji?? Van?k has updated the pull request incrementally with two
> additional commits since the last revision:
> >>
> >> - Moved listing of tests to separate method: listTests
> >> - -l description moved to constant
> >
> > jcstress-core/src/main/java/org/openjdk/jcstress/Options.java line 50:
> >
> >> 48: public class Options {
> >> 49:
> >> 50: private static final String LIST_OPTION_DESCRIPTION="List the
> available tests matching the requested settings, after all filters (like
> CPU count) are applied. In verbose mode it prints all real combinations
> which will run.";
> >
> > feel free to split this in two lines if you prefer, you are right on the
> readability aspect, anyways the concatenation would happen just once at the
> application startup
>
> thanx! Split.
>
> -------------
>
> PR Review Comment:
> https://git.openjdk.org/jcstress/pull/149#discussion_r1665155175
>
>
> ------------------------------
>
> Message: 3
> Date: Thu, 4 Jul 2024 06:08:29 GMT
> From: Ji?? Van?k <jvanek at openjdk.org>
> To: <jcstress-dev at openjdk.org>
> Subject: Re: RFR: CODETOOLS-7903748 - jcstress: Test list should honor
> concurrency settings [v4]
> Message-ID:
> <skvJ7gQ-Vv8REzHDZnDsrAWxEojWX26E8ssWb6ya5XA=.
> 2aa97ef4-5dcb-4d02-b7c6-55782fdc7cac at github.com>
>
> Content-Type: text/plain; charset=utf-8
>
> On Wed, 3 Jul 2024 19:21:08 GMT, PM <duke at openjdk.org> wrote:
>
> >> Ji?? Van?k has updated the pull request incrementally with two
> additional commits since the last revision:
> >>
> >> - Moved listing of tests to separate method: listTests
> >> - -l description moved to constant
> >
> >
> 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.
>
> 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 listig output format - I intentionally kept it semi
> parseabel - grep/sed on output, and readable enough: name info in {} ,
> all firelds with key id, and obvious arrays in [] without key.
> AFAICJ, the full json will lower the human readabiity.
> 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?
>
> -------------
>
> PR Review Comment:
> https://git.openjdk.org/jcstress/pull/149#discussion_r1665173411
>
>
> ------------------------------
>
> Message: 4
> Date: Thu, 4 Jul 2024 06:25:05 GMT
> From: Ji?? Van?k <jvanek at openjdk.org>
> To: <jcstress-dev at openjdk.org>
> Subject: Re: RFR: CODETOOLS-7903756 - jcstress shuld not pass
> debugging parameters to subproceses [v4]
> Message-ID:
> <MimRJngLQMOKSxr30nggGne8xyE0ONUdSZF9yTcdE1U=.
> 73657180-4fd6-4683-8b0e-ed10d5028d3e at github.com>
>
> Content-Type: text/plain; charset=utf-8
>
> > No longer passing any -agentlib to subprocesses, unless it is part of
> -jvmArgsPrepend, thus allowing to debug both jcstress itself or wrked vm as
> expected:
> >
> > java
> -agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5005 -jar
> tests-all/target/jcstress.jar -c 1 -jvmArgsPrepend
> "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=*:5006"
> >
> > works.
>
> Ji?? Van?k has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Removed unused imports
>
> -------------
>
> Changes:
> - all: https://git.openjdk.org/jcstress/pull/150/files
> - new:
> https://git.openjdk.org/jcstress/pull/150/files/23291cb3..6ffc620b
>
> Webrevs:
> - full: https://webrevs.openjdk.org/?repo=jcstress&pr=150&range=03
> - incr: https://webrevs.openjdk.org/?repo=jcstress&pr=150&range=02-03
>
> Stats: 3 lines in 2 files changed: 0 ins; 3 del; 0 mod
> Patch: https://git.openjdk.org/jcstress/pull/150.diff
> Fetch: git fetch https://git.openjdk.org/jcstress.git
> pull/150/head:pull/150
>
> PR: https://git.openjdk.org/jcstress/pull/150
>
>
> End of jcstress-dev Digest, Vol 71, Issue 6
> *******************************************
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/jcstress-dev/attachments/20240704/3b365686/attachment-0001.htm>
More information about the jcstress-dev
mailing list