Shenandoah GC not available in jdk11 AIX and JDK11 window32

[sophia-guo]

Smoke test builds shows that Shenandoah GC not available in jdk11 AIX and JDK11 window32, which is unexpected? #2114.

18:53:34  PASSED: testZGCAvailable
18:53:34  FAILED: testShenandoahAvailable
18:53:34  java.lang.AssertionError: Expected Shenandoah to be absent but it is present. expected [true] but found [false]
18:53:34  	at org.testng.Assert.fail(Assert.java:96)
18:53:34  	at org.testng.Assert.failNotEquals(Assert.java:776)
18:53:34  	at org.testng.Assert.assertTrue(Assert.java:44)
18:53:34  	at net.adoptopenjdk.test.FeatureTests.testShenandoahAvailable(FeatureTests.java:90)

https://ci.adoptopenjdk.net/job/build-scripts/job/jobs/job/jdk11u/job/jdk11u-windows-x86-32-hotspot_SmokeTests/1/console
https://ci.adoptopenjdk.net/job/build-scripts/job/jobs/job/jdk11u/job/jdk11u-aix-ppc64-hotspot_SmokeTests/2/console

[karianna]

https://wiki.openjdk.java.net/display/shenandoah/Main suggests that Win 32 should work but PPC may still be in development by SAP. I'm not overly concerned about the Win32 failing as I suspect that use case is 0.

[smlambert]

Description
If smoke tests fail, we block the running of AQA testing (as in no point to continue, because build/packaging is wrong).

In order to address this issue, we must make changes to the playlist.xml file in this repository.

A test is failing on Win32, possibly because some build config stuff is not being set, and that should get verified if it is the issue, and then fixed. In the meantime, we should exclude the test for win32 platform, but this issue should stay open until we resolve why its failing. We can do this by adding a disabled block to the test definition in the playlist file. Something like:

<disabled>
	<comment>https://github.com/AdoptOpenJDK/openjdk-build/issues/2566</comment>
	<plat>x86-32_windows</plat>
</disabled>

For AIX, because it is still in development, we could add <platformRequirements>^arch.ppc64</platformRequirements> to the test definition in the playlist.xml.

📋 Step by Step

To solve this issue and contribute a fix you should check the following step-by-step list. A more detailed documentation of the workflow can be found here. Note: that you do not need to add yourself to a Contributors.md file as outlined in the detailed instructions.

• Claim this issue: Comment below.
• Fork the repository in github by simply clicking the 'fork' button.
• Check out the forked repository
• Create a feature branch for the issue. We do not have any naming definition for branches.
• Commit your changes.
• Start a Pull Request.
• Done 👍 Ask in comments for a review :)
• If the reviewer find some missing pieces or a problem they will start a discussion with you and describe the next steps how the problem can be solved.
• You did it 🎉 We will merge the fix in the master branch.
• Thanks, thanks, thanks for being part of this project as an open source contributor ❤️

[sxa]

Hmmm my hazy recolletion was that it had only been implemented - at least initially - for x64 and aarch64. Does it work on Linux/ppc64le and Linux/s390x?

[smlambert]

Will presume the support table at https://wiki.openjdk.java.net/display/shenandoah/Main is accurate, so no.

[smlambert]

https://github.com/AdoptOpenJDK/openjdk-build/blob/master/sbin/build.sh#L79-L85

The win32 build job output has --with-jvm-features=shenandoahgc as a build script argument:

15:28:53  Running ./configure with arguments 'bash ./configure --verbose  --with-vendor-name=AdoptOpenJDK --with-vendor-url=https://adoptopenjdk.net/ --with-vendor-bug-url=https://github.com/AdoptOpenJDK/openjdk-support/issues --with-vendor-vm-bug-url=https://github.com/AdoptOpenJDK/openjdk-support/issues --without-version-opt --without-version-pre --with-version-build=8 --with-vendor-version-string=AdoptOpenJDK-11.0.11+8 --with-boot-jdk=/cygdrive/c/openjdk/jdk-10 --with-jvm-features=shenandoahgc --with-debug-level=release --with-native-debug-symbols=external --with-jvm-variants=client,server --with-cacerts-file=/cygdrive/e/jenkins/tmp/sbin/../security/cacerts  --disable-warnings-as-errors --disable-ccache --with-target-bits=32 --target=x86 --disable-ccache --with-toolchain-version=2017 '

[sxa]

@adamfarley Since you're still using a Windows laptop as far as I know are you able to take a look at this one?

[adamfarley]

I can take a look, sure. Will read.

[adamfarley]

I think the error message has been incorrectly interpreted. It says:

"Expected Shenandoah to be absent but it is present"

So the problem isn't that we're missing Shenandoah. The problem is that we have it, though the test was not expecting it.

Verifying this, further up we see:

"INFO: Detected 11.0.11.0 on WINDOWS/X86, expect Shenandoah to be present: false"

Will inspect the test. It could simply be that Shenandoah wasn't yet available on 32bit Windows during this smoke test's development. This website definitely implies that a 32bit Windows Shenandoah was planned.

[sophia-guo]

#2566 (comment)

Most of smoke tests are TestNG. We could also use disable to exclude specific test method testShenandoahAvailable(or EXCLUDE file enable by exclude transform utility in https://github.com/eclipse/openj9/tree/master/test/Utils/src/org/openj9/test/util ) to avoid disable the whole testCase Adopt_HS_FeatureTests.

[adamfarley]

Also, I've looked at the output from the aix test. I think "testShenandoahAvailable" passed, as it was correctly missing from the AIX build.

The test that failed on aix was "testJFRAvailable", which I don't believe to be connected to Shenandoah.

[adamfarley]

@sophia-guo - Please could you raise a seperate issue for the "testJFRAvailable" failure on AIX?

As for the win32 Shenandoah issue, I'm creating a PR so that we expect Shenandoah on Windows 32bit. I'm not seeing any intentional exclusion of Shenandoah on Windows 32bit in the backport change request, and the project webpage definitely seems to imply Windows 32bit can use Shenandoah if the right options have been set at build time.

Unless someone can tell me otherwise, I'm going to assume Windows 32bit was just skipped during smoke test pre-deployment testing.

[adamfarley]

@sophia-guo - Could you review my proposed fix here, and let me know what you think? My reasoning is here.

If it looks right to you, we can start moving through the process for PR merging during a release, as documented here.

Edit: Also realised this change will affect jdk15+ runs too, so I checked and the JDK16u win32 smoke tests are also seeing this issue, so we're fine. Two failures, one fix. :)

[sxa]

Thanks @adamfarley for investigating - your reasoning seems solid to me 👍

[smlambert]

Your proposed fix looks good @adamfarley - please create a PR for it (and we can follow the release merge process to decide if it will be merged during or after the release).

[adamfarley]

Okie dokie, will do. :)

[adamfarley]

Ok, PR: #2580

Unfortunately Linter isn't happy.

https://github.com/AdoptOpenJDK/openjdk-build/pull/2580/checks?check_run_id=2365074913

It's mainly my old friend, the 80 char line limit, and several alerts about every number in the file being a magic number(?).

To save me making many small changes to this file this close to the release, would it be ok if we skipped the linter check this once?

[smlambert]

I've created #2581 to scrutinize linter and discuss if should be updated to be more reasonable/realistic. I see precedence on skipping Linter check for other PRs recently. Will be good to set it to the right level of checking post release (and given the precedence, I would not block on this check... though will defer to 2nd reviewer's opinion).

[sophia-guo]

AIX JFR issue opened #2582, this can be close by #2580 .

[adamfarley]

Thanks Sophia. :)