Switch OpenJ9 builds on JDK11 to mixedref specs

[AdamBrousseau]

Mixed refs combines the regular and XL package
into one sdk. Also remove the deprecated XL
specs from jdk11. Once this is default across
all versions there will be more code that can
be cleaned up I presume.

Issue #2426

Signed-off-by: Adam Brousseau adam.brousseau88@gmail.com

[AdamBrousseau]

Note: I don't like the way ADDITIONAL_FILE_NAME_TAG is used to get the test job name. In the future once all OpenJ9 versions are changed to mixedref specs I assume we will not want any additional filename. However at that point we will probably switch back to the default test specs too, so it may be a moot point.

I have tested a version of this change on my own farm but I encourage a proper test be done here.

[sxa]

Is there a reason why we're adding in a suffix for this as opposed to just leaving the original one named as-is? This could have knock on effects elsewhere (API, installer) etc. and it would superficially seem easier to just keep the existing names.

[AdamBrousseau]

We're probably fine with making the "default" mixed on jdk11 right away. Which is why I reused the default specs. But we'll have to ensure we can support XL for other versions for the time being and that means launching *_mixed test builds. Agree it would be easier to change everything over all at once to cut down on temporary changes and additional work for other repos. I'm not sure if we want to wait.

cc @pshipton

Edit: I guess I didn't directly answer your question. We need the suffix (filename addition) in order to call the right test job name.

[pshipton]

My understanding is that once Adopt switches to mixed refs builds for jdk11, we can modify the jdk11 extensions to make mixed refs the default, and concurrently @llxia can help modify the testing for jdk11 so it always runs mixed ref testing. Then for jdk11 we won't need _mixed.

We don't want to wait until all versions are ready, at OpenJ9 the plan is to build mixed refs asap, and enable the remaining platforms as they are fixed. This exposes any problems earlier.

OpenJ9 current status:
jdk11 - all platforms switched to mixed refs already at OpenJ9
jdk8 - all but Windows are being tested and will switch asap, likely tomorrow if all goes well
jdk16 - will be tested but due to the upcoming release won't switch any platforms until all platforms are ready. Windows, AIX are not ready yet. It's possible Windows and AIX will be ready next week, or at least this month before the release.
jdk17 - will be tested (last) and platforms switched. Windows, AIX are not ready yet. AIX will likely be ready similarly to jdk16, but Windows may take longer.

[AdamBrousseau]

If the test auto detect could be configured to not fail we could continue to call the regular spec jobs.

[M-Davies]

run tests

[adoptopenjdk-github-bot]

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

[M-Davies]

@smlambert @andrew-m-leonard @karianna Need an approval for the PR tester

[smlambert]

PR tester approved.

Regarding testing, Lan and team already have a plan in place, with the understanding that we need to support all 3 types of builds for a while, and then at some point when we can drop the 2 and move completely to mixed it becomes the nonSuffix default (some of this to happen via TKG with a modes and spec cleanup).

[pshipton]

What Lan and I discussed today is dropping the 2 and moving completely to mixed on a version by version basis. Once this PR is merged we could do this for jdk11, which will help PR testing at the OpenJ9 project.

[pshipton]

I forgot about z/OS, which doesn't use mixed, but perhaps that platform can be accommodated.

[M-Davies]

run tests

[adoptopenjdk-github-bot]

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

[M-Davies]

run tests

[adoptopenjdk-github-bot]

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

[M-Davies]

PR tester failed due to unrelated causes. Lets merge this and see how they go?

[M-Davies]

@AdamBrousseau Can you rebase your branch to match master? I'd like to try something that might clear up #2446 Belay that, I managed to do so on a seperate PR

[sxa]

What Lan and I discussed today is dropping the 2 and moving completely to mixed on a version by version basis. Once this PR is merged we could do this for jdk11, which will help PR testing at the OpenJ9 project.

Superficially that sounds like a preferable option if it can be done, the only question being whether if we switched a release over without the _mixed suffix (i.e. just using the base name) would the tests be able to identify and use it properly?

[andrew-m-leonard]

@AdamBrousseau needs re-basing on master now that parameterized generation PR has merged, which is why the "pipeline-build-check" is failing

[andrew-m-leonard]

Looks good, just needs rebasing on master so we can "run tests"

[M-Davies]

Looks good, just needs rebasing on master so we can "run tests"

May as well hold off on rebasing until #2450 and #2452 are merged as they will fix the PR tester issues

[adoptopenjdk-github-bot]

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

[AdamBrousseau]

What Pete is talking about sounds like the end goal. I was under the impression we would have a temporary state of 3 build types. If we're going full change-over, even it's its one version at a time, I'm not sure we need this PR. I think it would only be an extensions change and a test change.

[sxa]

we'll have to coordinate a change to the openjdk11 extensions to make mixed builds the default.

That's the other option I suppose and it wouldn't need a PR from us, other than to remove the XL ones later. I'd possibly be a little more comfortable than putting a PR in to switch them on for now until we're certain it doesn't cause any side effects (ie.e. it's easy to revert)

[pshipton]

So the options are:

• Wait for OpenJ9 & test to make mixedrefs the default. The XL builds at Adopt should be disabled at that point as the non-XL builds are sufficient. If Adopt needs to build without mixedrefs then --with_mixedrefs=no can be used, but no testing would work.
• Merge this to get some mixedrefs builds at Adopt in advance of OpenJ9 & test changing the default. OpenJ9 changing the default afterwards won't change anything at Adopt, but this change will be redundant at that point.

[pshipton]

Correction to my previous comment. Although --with_mixedrefs=no could be used to build, I believe the testing wouldn't work because it would be hard coded to expect mixedref builds.

[AdamBrousseau]

I don't think this works as-is. It seems additionalFileNameTag is expected to be a string only and doesn't support map. I will see if I can add the required changes.

[AdamBrousseau]

Updated change to allow additionalFileNameTag to be a map of strings. There are still 2 more pieces to fix (See TODO).
@M-Davies Do you have any guidance on these?

[sxa]

So the options are:

I think it needs inpiut from test as to what the implications are for each opytion - it's like;ly a matter of whether the "end game" of "move completly to mixed" in @smlambert 's statement (repeated below) can be done on a release-by-release basis.

Regarding testing, Lan and team already have a plan in place, with the understanding that we need to support all 3 types of builds for a while, and then at some point when we can drop the 2 and move completely to mixed it becomes the nonSuffix default (some of this to happen via TKG with a modes and spec cleanup).

[M-Davies]

Updated change to allow additionalFileNameTag to be a map of strings. There are still 2 more pieces to fix (See TODO).
@M-Davies Do you have any guidance on these?

Maybe you could try reading the additionalFileNameTag again and using that value to determine the estimated key? https://github.com/AdoptOpenJDK/openjdk-build/pull/2441/files#diff-1e179c7032cb39fa5e9bc1b22c951ef178a1320770efdcf78b21ffae072f60f7R463

Sidenote, you should also implement the changes you have made to ADDITIONAL_FILENAME_TAG in config_regeneration.groovy too, otherwise the downstream jobs won't pick up the new config https://github.com/AdoptOpenJDK/openjdk-build/pull/2441/files#diff-1e179c7032cb39fa5e9bc1b22c951ef178a1320770efdcf78b21ffae072f60f7R452

[gdams]

@AdamBrousseau are you happy to move this PR over to ci-jenkins-pipelines? I can move it over if you prefer?

[AdamBrousseau]

Can it be moved with the comments? I think the discussion is important.

[llxia]

From the testing perspective, if we want to leave the original test build named as-is and switch to JDK11 mixedref SDKs, I think we can simply set PLATFORM parameter to ***_mixed in test builds for openj9 JDK11 during test execution https://github.com/AdoptOpenJDK/openjdk-build/blob/d7780e2ab5afaceb3a1c63fc0621025431963485/pipelines/build/common/openjdk_build_pipeline.groovy#L273

And in the build script, we should remove _xl . No other change is needed.
Once we fully switched to mixedred, we can remove the above code of setting the PLATFORM during test execution.

@smlambert what do you think about this approach?

[smlambert]

re: #2441 (comment) and the discussion in the OpenJ9 community call today, I think that approach makes sense.

[AdamBrousseau]

Given that the amended code change will be a different approach and we're moving repos I'll open a new PR on the ci repo and link to this one for tracking purposes.