Now building also jcstress-tests-all-20240222.jar 20220908.jar #1020

judovana · 2024-04-25T16:54:08Z

Those are based on closest (which were actually that day last) commit hashes. junit tests are passing.

In addition shortened hash to 7 latters and uses jdk17 to tip

Should be fixing #1016

Those are based on closest (which were actually that day last) commit hashes. junit tests are passing. In addtion shortened hash to 7 latters and uses jdk17 to tip

github-actions · 2024-04-25T16:54:22Z

Thank you for creating a pull request!

Please check out the information below if you have not made a pull request here before (or if you need a reminder how things work).

Code Quality and Contributing Guidelines

If you have not done so already, please familiarise yourself with our Contributing Guidelines and Code Of Conduct, even if you have contributed before.

Tests

Github actions will run a set of jobs against your PR that will lint and unit test your changes. Keep an eye out for the results from these on the latest commit you submitted. For more information, please see our testing documentation.

In order to run the advanced pipeline tests (executing a set of mock pipelines), it requires an admin to post run tests on this PR.
If you are not an admin, please ask for one's attention in #infrastructure on Slack or ping one here.
To run full set of tests, use "run tests"; a subset of tests on specific jdk version, use "run tests quick 11,21"

and unified names of output jars

karianna · 2024-04-30T03:47:03Z

tools/code-tools/jcstress.sh


- echo "Resetting repo back to master"
- git checkout master
+ # 20240222


What is the significance of that date? Is that a tag for a major version?

As I see it, there is no significance. Unluckily there is also no tag. Double unluckily jcstress tags extremely rarely:(

As described here: adoptium/aqa-tests#5261 ; the jcstres in aqavit have heavily unclear binaries to execute:

The getDepenencies is pulling jcstress-tests-all-20240222.jar'
https://github.com/adoptium/TKG/blob/master/scripts/getDependencies.pl#L182
in addition, it is pulling it from personal server

and

however, the https://github.com/adoptium/aqa-tests/blob/master/system/jcstress/playlist.xml
is using jcstress-tests-all-20220908.jar

Those are not base don any tag, nor hash. They will be reused in the cstress runner, so the https://github.com/adoptium/TKG/blob/master/scripts/getDependencies.pl#L182 and https://github.com/adoptium/aqa-tests/blob/master/system/jcstress/playlist.xml are in harmony.

I think first step will be to let the getDependencies.pl to download 20220908, and use that in playlist.xml. Then to bump them both to 20240222 (as this is quite fresh). There may come som if jdk 8/11/...22 swithc as is done with jtrregs. This is stil to be decided. For now, I wanted to prepare binareis to use.

Thanx for bringing it up. If you have any top level hints, dont hesitate to write them to #1016 and adoptium/aqa-tests#5261.
Actually.. any hints welcomed :) Thanx for eyball!

@smlambert - I think I need your guidance here :-). Which version(s) of jcstress does the testing group need? I read the other issues but I'm still unsure...

as any builder, we need tip and latest release for sure. I heavily recommend to not get stick to latest release, as they are not fluently released, and to verify jcstress failure you have to try something newer, always, even tip from time to time. So A middle path is to pick up something known to be stable on the fly, and starting using that. 20240202 is pretty fresh, and known to be stable, so very good candidate.

smlambert · 2024-04-30T11:18:53Z

We do not need a build from that date. We need/want to use tagged or released builds. In order to do that, we need to 'test' that an appropriately recent tagged build can run the test targets we have, without major errors.

So indeed, please do not bother building untagged/untracked versions of jcstress. I do not have time to review this PR today as I am on 'personal time off'. Please do not rush into merging anything. There is all the time in the world to get things right and yet takes no time at all to do the wrong thing.

judovana · 2024-04-30T12:18:05Z

This PR is not in rush for sure. Whatever we agree should be built, will be built.

However I think tagged builds are not an option. jcstress team (read shade) tags somehow randomly, and last tag is pretty old.
Also every time I reported an issue, first order was update jcstress, and if not to some particular commit,then to head. And indeed it worked in 3/4 of cases.
Similarly the https://mvnrepository.com/artifact/org.openjdk.jcstress is doing that. IIRC, I was trying to do maven releases more often, and I still have permissions to do that, but that is still jsut random local build. Same counts for JMH. And you still need one of project owners to tag the repo, and that may be show-stopper pretty often.

Also I doubt we wish to go with tip, as that can indeed change dramatically without warning. From that point of view the 20240222 is pretty good option as it is known to be stable and is really fresh.

I was following adoptium/aqa-tests#5261 where you gave me green light, unluckily only during voice chat, and it literally says:

improve https://ci.adoptium.net/view/Dependencies/job/dependency_pipeline/lastSuccessfulBuild/artifact/jcstress/ so it builds jcstress-tests-all-20220908.jar and jcstress-tests-all-20240222.jar and tip

Which is what this PR is doing (tip was already there).

If you reconsidered, it is ok and I will follow for sure, I do not have intentions to break anything, but I would like to have aqa-tests jcstress build on better foundation then on random jcstress-tests-all-20220908.jar which was downloaded ages ago and can no longer be recreated. (Or I have not found it).
So indeed the builder should be building 20220908 - so the https://github.com/adoptium/aqa-tests/blob/master/system/jcstress/playlist.xml and https://github.com/adoptium/TKG/blob/master/scripts/getDependencies.pl#L182 can immediately reuse that. Then it should be able to pick up 20240222 moreover out of the box. Maybe except jdk8 and maybe only on demand.. that will follow.

Generally it is good we had this conversation. I realised, that all above is valid also for JMH, and that jcstres palylist and/or https://github.com/adoptium/TKG/blob/master/scripts/getDependencies.xml have to be able to work - on demand only! - with jcstress tip, as that is waht jcstress people will always ask us to do first.

No rush from me. Do not spoil your time off. You made yourself clear you wish to double check that, and I'm greatful for it.

judovana · 2024-05-09T13:17:51Z

Looking to adoptium/aqa-tests#5278 (and seeing adoptium/aqa-tests#5270 work ok) This shold be building only 20240222, and lets say with "correct" name of jcstress-tests-all-20240222.jar ?

sophia-guo · 2024-05-22T19:54:38Z

Currently aqa-tests is grabing jcstress from personal server so this PR won't affect aqa-tests. I think 20240222 is enough ( no need to build 20220908.jar).

What's the difference of building the jar with jdk17 and jdk11? Do we need both version? Once this is in we can update aqa-tests.

judovana · 2024-05-23T13:36:38Z

Currently aqa-tests is grabing jcstress from personal server so this PR won't affect aqa-tests. I think 20240222 is enough ( no need to build 20220908.jar).

Right and right. I will drop the 2022 version as the 2024 is already "iun production" (was not when I strarted thi sPR)
But immediately once it is in I will switch the wget of it as descibed in adoptium/aqa-tests#5261 from personal server, to result of this build (latestSucesfullBuild jcstress/Artifacts)

What's the difference of building the jar with jdk17 and jdk11? Do we need both version? Once this is in we can update aqa-tests.

That appeared to be wrong assumption. I will swithc it all to jdk8.At least we will know, when jdk8 is not worthy anymore.

judovana · 2024-05-23T15:03:02Z

Please double check the naming of main tarball.
I'm using: jcstress.jar, jcstress-0.16.jar jcstress-20240222.jar, jcstress-somehash.jar and jcstress-tip.jar, (last two and first two are hardlink). The someHash one is now jcstress-d6a81c3.jar

Where this is more aligned to how maven is generating the jars, current usage in adoptium is jcstress-tests-all-20240222.jar which is slightly more describing waht directroy the tests are from. However I think it is not necessary to highlight because it is the only jar produced during default build. Note, that the old naming is used in both current generators (see adoptium/aqa-tests#5278). But that woudl be super easy to adjust onc ethis is in. TY!

sophia-guo · 2024-05-23T18:20:43Z

That appeared to be wrong assumption. I will swithc it all to jdk8.At least we will know, when jdk8 is not worthy anymore.

Not sure what do you mean switch it all to jdk8. The project mentioned The project requires JDK 11+ to build. https://github.com/openjdk/jcstress ?

judovana · 2024-05-23T18:45:50Z

Sorry, I overwrote. See the PR, I switched to 11. I ment 11. My wrong for that "typo"/

sophia-guo · 2024-05-23T18:59:41Z

https://ci.adoptium.net/view/Dependencies/job/dependency_pipeline/1186/artifact/jcstress/

Are jcstress-0.16.jar, jcstress-20240222.jar, jcstress-d6a81c3.jar different? If not we don't need to keep all of them?

Could you @judovana make the PR ready for review if it's ready?

judovana · 2024-05-24T06:00:52Z

The linking follows all other ci-pipellines buildjobs.

tip have hash in name, and is linked as tip (jcstress-d6a81c3.jar +jcstress-tip.jar)
latest release have release in name and is linked as "that" (jcstress-0.16.jar + jcstress.jar)
jcstress-20240222.jar as artificial jar for internal purposes have no linking

The goal is to have predictable names for pernament links, and still haveing clear what is waht.

The jcstress-20240222.jar is now very similar to jcstress-d6a81c3.jar because only few commits was added in meantime, but it is ill idea to have internal testing agasint tip.

sophia-guo · 2024-05-27T15:26:36Z

So jcstress.jar, jcstress-d6a81c3.jar and jcstress-tip.jar are same? For now I think tip, latest release, jcstress-20240222.jar are enough as long as the sha is provided.

sophia-guo · 2024-05-27T15:27:31Z

tools/code-tools/jcstress.sh

 jdk11=$(readlink -f $(find ${jvm_dir} -maxdepth 1 | sort | grep -e java-11- -e jdk-11 | head -n 1))
+ jdk17=$(readlink -f $(find ${jvm_dir} -maxdepth 1 | sort | grep -e java-17- -e jdk-17 | head -n 1))


Do we still need this?

We dont. I can drop it if you insists. But I'm 51:49 to keep it. It is good to list individual jDKS in buildroot. Especially in case, that something will nto honour JAVA_HOME, it will ebcome very usefull.

Are we missing Java 21 here as well?

What do you mean miss java 21? The container have jdk8, jdk11, jdk17 and also jdk21 iirc. Jcsress since some 2023 needs at elast jdk11 to build with. I'm checking the hdk versiosn just for case the build goes wrong. I can check the 21 too if you wish.

the upstream is correctlysetting source/version/release javc flags so to build each version by different jdk was just candy on the top. Now all three jcstresses (latest release, tip, and "latest used by us" ) are built by jdk11. If you wish, I have no objections to build tip by soemthign newer. :) ty!

It sounds like you only need to check for the existence of 11+ in that case? It seems over kill to check for each LTS version...

judovana · 2024-05-27T15:42:36Z

So jcstress.jar, jcstress-d6a81c3.jar and jcstress-tip.jar are same? For now I think tip, latest release, jcstress-20240222.jar are enough as long as the sha is provided.

no jcstress-d6a81c3.jar and jcstress-tip.jar are same, and then jcstress.jar anf (jcstress-0.16.jar as I wrote above.

All other ci-tools are keeping this convention. So I would keep it.

judovana · 2024-05-27T15:43:31Z

tyvm! for approve!

sophia-guo · 2024-06-10T12:46:01Z

@karianna any suggestion?

judovana · 2024-06-10T12:48:07Z

@karianna Hello, can you reevaluate?

karianna

Need @smlambert to confirm versions we want supported.

smlambert · 2024-06-14T12:34:15Z

Yes, in absence of a stable release tag to build off of, we proceed with this approach. If jcstress begins to employ release tagging in future, we can adjust.

Now buildign also jcstress-tests-all-20240222.jar 20220908.jar

7033de7

Those are based on closest (which were actually that day last) commit hashes. junit tests are passing. In addtion shortened hash to 7 latters and uses jdk17 to tip

github-actions bot added the testing label Apr 25, 2024

Removed duplicated code

b0d5086

and unified names of output jars

karianna reviewed Apr 30, 2024

View reviewed changes

judovana marked this pull request as draft April 30, 2024 12:20

judovana mentioned this pull request May 2, 2024

Update jcstress version adoptium/aqa-tests#5270

Merged

judovana mentioned this pull request May 9, 2024

return accidentally removed -e from scripts/testenv/testenvSettings.sh adoptium/aqa-tests#5301

Closed

Merge branch 'master' into moreJcStress

50e9428

Merge branch 'master' into moreJcStress

9dd1e00

Removed 20220908 version

aad5aa6

judovana marked this pull request as ready for review May 24, 2024 05:56

Merge branch 'master' into moreJcStress

22694ec

sophia-guo reviewed May 27, 2024

View reviewed changes

sophia-guo approved these changes May 27, 2024

View reviewed changes

judovana requested a review from karianna June 10, 2024 12:47

karianna changed the title ~~Now buildign also jcstress-tests-all-20240222.jar 20220908.jar~~ Now building also jcstress-tests-all-20240222.jar 20220908.jar Jun 12, 2024

karianna reviewed Jun 12, 2024

View reviewed changes

smlambert approved these changes Jun 14, 2024

View reviewed changes

smlambert merged commit 5866850 into adoptium:master Jun 14, 2024
8 checks passed

smlambert mentioned this pull request Jun 14, 2024

Regenerated jcstress playlist for jcstress and pushed its generator adoptium/aqa-tests#5317

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Now building also jcstress-tests-all-20240222.jar 20220908.jar #1020

Now building also jcstress-tests-all-20240222.jar 20220908.jar #1020

judovana commented Apr 25, 2024 •

edited

Loading

github-actions bot commented Apr 25, 2024

karianna Apr 30, 2024

judovana Apr 30, 2024 •

edited

Loading

karianna Jun 12, 2024

judovana Jun 13, 2024

smlambert commented Apr 30, 2024

judovana commented Apr 30, 2024 •

edited

Loading

judovana commented May 9, 2024

sophia-guo commented May 22, 2024

judovana commented May 23, 2024

judovana commented May 23, 2024

sophia-guo commented May 23, 2024

judovana commented May 23, 2024

sophia-guo commented May 23, 2024

judovana commented May 24, 2024

sophia-guo commented May 27, 2024

sophia-guo May 27, 2024

judovana May 27, 2024

karianna Jun 12, 2024

judovana Jun 13, 2024

karianna Jun 14, 2024

judovana commented May 27, 2024

judovana commented May 27, 2024

sophia-guo commented Jun 10, 2024

judovana commented Jun 10, 2024

karianna left a comment

smlambert commented Jun 14, 2024

		jdk11=$(readlink -f $(find ${jvm_dir} -maxdepth 1 \| sort \| grep -e java-11- -e jdk-11 \| head -n 1))
		jdk17=$(readlink -f $(find ${jvm_dir} -maxdepth 1 \| sort \| grep -e java-17- -e jdk-17 \| head -n 1))

Now building also jcstress-tests-all-20240222.jar 20220908.jar #1020

Now building also jcstress-tests-all-20240222.jar 20220908.jar #1020

Conversation

judovana commented Apr 25, 2024 • edited Loading

github-actions bot commented Apr 25, 2024

Thank you for creating a pull request!

Code Quality and Contributing Guidelines

Tests

Choose a reason for hiding this comment

judovana Apr 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smlambert commented Apr 30, 2024

judovana commented Apr 30, 2024 • edited Loading

judovana commented May 9, 2024

sophia-guo commented May 22, 2024

judovana commented May 23, 2024

judovana commented May 23, 2024

sophia-guo commented May 23, 2024

judovana commented May 23, 2024

sophia-guo commented May 23, 2024

judovana commented May 24, 2024

sophia-guo commented May 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

judovana commented May 27, 2024

judovana commented May 27, 2024

sophia-guo commented Jun 10, 2024

judovana commented Jun 10, 2024

karianna left a comment

Choose a reason for hiding this comment

smlambert commented Jun 14, 2024

judovana commented Apr 25, 2024 •

edited

Loading

judovana Apr 30, 2024 •

edited

Loading

judovana commented Apr 30, 2024 •

edited

Loading