8358756: [s390x] Test StartupOutput.java crash due to CodeCache size

[offamitkumar]

There isn't enough initial cache present which can let the interpreter mode run freely. So before even we reach to the compiler phase and try to bail out, in case there isn't enough space left for the stub compilation, JVM crashes. Idea is to increase the Initial cache size and make it enough to run interpreter mode at least.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8358756: [s390x] Test StartupOutput.java crash due to CodeCache size (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25741/head:pull/25741
$ git checkout pull/25741

Update a local copy of the PR:
$ git checkout pull/25741
$ git pull https://git.openjdk.org/jdk.git pull/25741/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25741

View PR using the GUI difftool:
$ git pr show -t 25741

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25741.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back amitkumar! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@offamitkumar The following label will be automatically applied to this pull request:

• hotspot-compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (12b60494)
• 00: Full (bcf22368)

[dafedafe]

Thanks @offamitkumar.
The idea behind the PR that changed this is that it would check randomly around the amount of code cache that would be just enough for the compilers to start (or not). So, before that PR it would sometimes crash instead of terminating gently. Does adding 800k to the initial code cache for s390 do that? Did you try before that PR (or temporarily reverting it) to see if it crashes?

[dafedafe]

It is just a stylistic issue but I'd rather adapt the fist constant (800) depending on the platform with something like:
int initialCodeCacheSizeInKb = minInitialSize + rand.nextInt(400);

and minInitialSize set depending on the platform before the loop.

[offamitkumar]

Thanks @offamitkumar. The idea behind the PR that changed this is that it would check randomly around the amount of code cache that would be just enough for the compilers to start (or not). So, before that PR it would sometimes crash instead of terminating gently. Does adding 800k to the initial code cache for s390 do that? Did you try before that PR (or temporarily reverting it) to see if it crashes?

Just for my understanding. Even if test passes we still want to see this warning:

[warning][codecache] CodeCache is full. Compiler has been disabled.

Before the PR, I don't test crashing or even producing this warning. Even with my changes same behaviour is going on.

[openjdk]

@offamitkumar this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout testfix
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[offamitkumar]

Thanks @offamitkumar. The idea behind the PR that changed this is that it would check randomly around the amount of code cache that would be just enough for the compilers to start (or not). So, before that PR it would sometimes crash instead of terminating gently. Does adding 800k to the initial code cache for s390 do that? Did you try before that PR (or temporarily reverting it) to see if it crashes?

I tried to see the output from all thread, and modified the test a bit to verify that:

java.lang.RuntimeException: 'CodeCache is full' found in stdout
	at jdk.test.lib.process.OutputAnalyzer.stdoutShouldNotContain(OutputAnalyzer.java:337)
	at compiler.startup.StartupOutput.main(StartupOutput.java:76)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
	at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:335)
	at java.base/java.lang.Thread.run(Thread.java:1474)

Above crash I got, once i modified the test case and then ran it:

diff --git a/test/hotspot/jtreg/compiler/startup/StartupOutput.java b/test/hotspot/jtreg/compiler/startup/StartupOutput.java
index 68cfaece2a5..02f3437c27b 100644
--- a/test/hotspot/jtreg/compiler/startup/StartupOutput.java
+++ b/test/hotspot/jtreg/compiler/startup/StartupOutput.java
@@ -73,7 +73,7 @@ public static void main(String[] args) throws Exception {
         for (int i = 0; i < 200; i++) {
             out = new OutputAnalyzer(pr[i]);
             // The VM should not crash but will probably fail with a "CodeCache is full. Compiler has been disabled." message
-            out.stdoutShouldNotContain("# A fatal error");
+            out.stdoutShouldNotContain("CodeCache is full");
             exitCode = out.getExitValue();
             if (exitCode != 1 && exitCode != 0) {
                 throw new Exception("VM crashed with exit code " + exitCode);

So I think compiler is bailing out with current changes. Please let me know if this is incorrect or any other question you have.

[dafedafe]

Just for my understanding. Even if test passes we still want to see this warning:

[warning][codecache] CodeCache is full. Compiler has been disabled.

The test passes with and without that message. When the randomly chosen amount of code cache is not enough to start the compiler(s) it should print that message, when it is enough to start both compilers, you don't see that message.
The important thing is that there is no crash when compilers are trying to reserve code cache (they should be just shut down).

[offamitkumar]

Just for my understanding. Even if test passes we still want to see this warning:

[warning][codecache] CodeCache is full. Compiler has been disabled.

The test passes with and without that message. When the randomly chosen amount of code cache is not enough to start the compiler(s) it should print that message, when it is enough to start both compilers, you don't see that message. The important thing is that there is no crash when compilers are trying to reserve code cache (they should be just shut down).

What I wanted to verify with above expected crash is that current number are not enough for the compilers and we saw the output is containing that "Codecache is full" message.

Now we can claim that this message is there in the log output and test passes successfully.