8290211: jdk/internal/vm/Continuation/Fuzz.java failed with "AssertionError: Failed to compile int Fuzz.com_int(int,int) in 5000ms" #9844

dcubed-ojdk · 2022-08-11T20:02:06Z

A trivial fix so that Continuation/Fuzz.java honors the timeoutFactor JTREG setting
when waiting for a compilation to finish.

This fix is being tested in my jdk-20+10 stress testing run.

The usual Mach5 timeoutFactor is 4.0 with slower configurations using a timeoutFactor
of 10.0. In my stress testing, I use release-bits: 4.0, fastdebug-bits: 6.0 and slowdebug-bits: 12.0.

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-8290211: jdk/internal/vm/Continuation/Fuzz.java failed with "AssertionError: Failed to compile int Fuzz.com_int(int,int) in 5000ms"

Reviewers

Leonid Mesnik (@lmesnik - Reviewer) ⚠️ Review applies to bd9be934
Alan Bateman (@AlanBateman - Reviewer) ⚠️ Review applies to bd9be934
Jie Fu (@DamonFool - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9844

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

Using diff file

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

…nError: Failed to compile int Fuzz.com_int(int,int) in 5000ms"

bridgekeeper · 2022-08-11T20:02:46Z

👋 Welcome back dcubed! 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 · 2022-08-11T20:04:43Z

@dcubed-ojdk The following label will be automatically applied to this pull request:

core-libs

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.

dcubed-ojdk · 2022-08-11T20:05:48Z

/label add hotspot-runtime

openjdk · 2022-08-11T20:06:28Z

@dcubed-ojdk
The hotspot-runtime label was successfully added.

mlbridge · 2022-08-11T20:10:37Z

Webrevs

01: Full - Incremental (cd61c316)
00: Full (bd9be934)

openjdk · 2022-08-12T03:45:35Z

@dcubed-ojdk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8290211: jdk/internal/vm/Continuation/Fuzz.java failed with "AssertionError: Failed to compile int Fuzz.com_int(int,int) in 5000ms"

Reviewed-by: lmesnik, alanb, jiefu

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 92 new commits pushed to the master branch:

07c9ba7: 8292686: runtime/cds/appcds/TestWithProfiler.java SIGSEGV in TableStatistics ctr
235151e: 8292676: Remove two kerberos tests from problem list
df5209e: 8292683: Remove BadKeyUsageTest.java from Problem List
74d3330: 8292682: Code change of JDK-8282730 not updated to reflect CSR update
57aac2a: 8290981: Enable CDS for zero builds
6a8a531: 8292564: x86: Remove redundant casts in Assembler usages
7244dd6: 8292267: Clean up synchronizer.hpp
1f3578b: 8292446: Make TableRateStatistics optional in CHT
45dec48: 8292592: JFR test TestNative is not reliable due to low rate of sampling.
58aae34: 8292576: Improve wording of AccessFlag-related specs
... and 82 more: https://git.openjdk.org/jdk/compare/7ea9ba1f6c18ace5aa0896ab8676926fdc0d64ea...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

DamonFool · 2022-08-12T04:03:56Z

test/jdk/jdk/internal/vm/Continuation/Fuzz.java

@@ -78,7 +78,8 @@ public class Fuzz implements Runnable {
    static final boolean RANDOM  = true;
    static final boolean VERBOSE = false;

-    static final int COMPILATION_TIMEOUT = 5_000; // ms
+    static float timeoutFactor = Float.parseFloat(System.getProperty("test.timeout.factor", "1.0"));


How about something like this?

System.getProperty("test.timeout.factor", "5.0")

Or

static final int COMPILATION_TIMEOUT = max((int)(1_000 * timeoutFactor), 5_000); // ms

System.getProperty("test.timeout.factor", "5.0")

Definitely not. Traditional default value for no specified timeoutFactor value is 1.0.

static final int COMPILATION_TIMEOUT = max((int)(1_000 * timeoutFactor), 5_000); // ms

So you're trying to make sure we have a minimum COMPILATION_TIMEOUT value of 5 seconds,
but I'm not sure why you want that.

static final int COMPILATION_TIMEOUT = (int)(5_000 * timeoutFactor); // ms

i.e., a default value * timeoutFactor is the usual way to do this. I've seen some
folks use a timeoutFactor of 0.5 on really fast machines, but that wouldn't work
with your minimum COMPILATION_TIMEOUT value of 5 seconds above.

dcubed-ojdk

@lmesnik, @DamonFool and @AlanBateman - Thanks for the reviews.

dcubed-ojdk · 2022-08-12T16:20:50Z

test/jdk/jdk/internal/vm/Continuation/Fuzz.java

@@ -78,7 +78,8 @@ public class Fuzz implements Runnable {
    static final boolean RANDOM  = true;
    static final boolean VERBOSE = false;

-    static final int COMPILATION_TIMEOUT = 5_000; // ms
+    static float timeoutFactor = Float.parseFloat(System.getProperty("test.timeout.factor", "1.0"));
+    static final int COMPILATION_TIMEOUT = (int)(1_000 * timeoutFactor); // ms


I was really hoping that 1_000 * timeoutFactor would do the trick,
but stress run-#1 on my macosx-aarch64 machine had a couple of
sub-test timeouts at 12 seconds (1_000 * 12.0). My timeoutFactor
for slowdebug is 12.0. My linux-x64 machine passed stress run-#1
without any failures.

I've switched the value from 1_000 back to 5_000 for the next two
stress runs on both machines.

dcubed-ojdk · 2022-08-12T16:26:03Z

test/jdk/jdk/internal/vm/Continuation/Fuzz.java

@@ -78,7 +78,8 @@ public class Fuzz implements Runnable {
    static final boolean RANDOM  = true;
    static final boolean VERBOSE = false;

-    static final int COMPILATION_TIMEOUT = 5_000; // ms
+    static float timeoutFactor = Float.parseFloat(System.getProperty("test.timeout.factor", "1.0"));


System.getProperty("test.timeout.factor", "5.0")

Definitely not. Traditional default value for no specified timeoutFactor value is 1.0.

static final int COMPILATION_TIMEOUT = max((int)(1_000 * timeoutFactor), 5_000); // ms

So you're trying to make sure we have a minimum COMPILATION_TIMEOUT value of 5 seconds,
but I'm not sure why you want that.

static final int COMPILATION_TIMEOUT = (int)(5_000 * timeoutFactor); // ms

i.e., a default value * timeoutFactor is the usual way to do this. I've seen some
folks use a timeoutFactor of 0.5 on really fast machines, but that wouldn't work
with your minimum COMPILATION_TIMEOUT value of 5 seconds above.

dcubed-ojdk · 2022-08-12T16:29:00Z

@lmesnik, @DamonFool and @AlanBateman - Thanks for the reviews.

DamonFool · 2022-08-12T23:42:47Z

So you're trying to make sure we have a minimum COMPILATION_TIMEOUT value of 5 seconds,
but I'm not sure why you want that.

With the current patch, COMPILATION_TIMEOUT may become less than 5_000, which may lead to more failures on various platforms.
That's why I suggest let COMPILATION_TIMEOUT >= 5_000, which won't make things worse.

Thanks.

DamonFool · 2022-08-12T23:44:26Z

Definitely not. Traditional default value for no specified timeoutFactor value is 1.0.

Okay, you're right.
This is because the timeoutFactor may be used by other tests too.

…actor.

dcubed-ojdk · 2022-08-19T19:51:13Z

Sorry for the delay in getting back to this PR. I've been focused on GateKeeping issues instead.

This latest update:

Scale the original COMPILATION_TIMEOUT value of 5 seconds by timeoutFactor.

makes the test happy on my linux-x64 stress runs. My macosx-aarch64 stress runs
are still not happy, but I'm still gathering data on those runs. I haven't found a
COMPILATION_TIMEOUT initial value that works every time yet. The values I've
tried so far:

1_000 - the original value in the PR
5_000 - the original value in the test before I changed it
10_000 - simple doubling
20_000 - simple doubling again, testing this value now and thru the weekend

I'm inclined to move ahead with the 5_000 value scaled by timeoutFactor. I think
I need to spend some time to determine why macosx-aarch64 is so much slower
than linux-x64 for this test.

DamonFool

LGTM

Thanks for the update.

dcubed-ojdk · 2022-08-20T02:38:15Z

@DamonFool - Thanks for the re-review!

dcubed-ojdk · 2022-08-22T19:28:30Z

/integrate

openjdk · 2022-08-22T19:31:01Z

Going to push as commit 54843b7.
Since your change was applied there have been 117 commits pushed to the master branch:

8a0c3e5: 8292261: adjust timeouts in JLI GetObjectSizeIntrinsicsTest.java
8e8ee4b: 8292596: Make SymbolHashMap a ResourceHashtable
aa9b8f0: 8292043: Incorrect decoding near EOF for stateful decoders like UTF-16
f95ee79: 8292566: Add reference to the java.nio.file package in java.nio package documentation
45c3e89: 8292316: Tests should not rely on specific JAR file names (jpackage)
db77227: 8282684: Obsolete UseContainerCpuShares and PreferContainerQuotaForCPUCount flags
256b523: 8292381: java/net/httpclient/SpecialHeadersTest.java fails with "ERROR: Shutting down connection: HTTP/2 client stopped"
e561933: 8292623: Reduce runtime of java.io microbenchmarks
dcd7802: 8292708: Rename G1ParScanThreadState::flush to flush_stats
16593cf: 8292717: Clean up checking of testing requirements in configure
... and 107 more: https://git.openjdk.org/jdk/compare/7ea9ba1f6c18ace5aa0896ab8676926fdc0d64ea...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-08-22T19:31:22Z

@dcubed-ojdk Pushed as commit 54843b7.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

dcubed-ojdk · 2022-08-22T19:46:18Z

jdk/internal/vm/Continuation/Fuzz.java* passed 12 times (2 sub-tests x 6 configs)
in two different Mach5 Tier1 job sets.

8290211: jdk/internal/vm/Continuation/Fuzz.java failed with "Assertio…

bd9be93

…nError: Failed to compile int Fuzz.com_int(int,int) in 5000ms"

openjdk bot added the core-libs core-libs-dev@openjdk.org label Aug 11, 2022

dcubed-ojdk marked this pull request as ready for review August 11, 2022 20:06

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Aug 11, 2022

openjdk bot added the rfr Pull request is ready for review label Aug 11, 2022

lmesnik approved these changes Aug 12, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Aug 12, 2022

DamonFool reviewed Aug 12, 2022

View reviewed changes

AlanBateman approved these changes Aug 12, 2022

View reviewed changes

dcubed-ojdk commented Aug 12, 2022

View reviewed changes

Scale the original COMPILATION_TIMEOUT value of 5 seconds by timeoutF…

cd61c31

…actor.

DamonFool approved these changes Aug 19, 2022

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Aug 22, 2022

openjdk bot closed this Aug 22, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Aug 22, 2022

dcubed-ojdk deleted the JDK-8290211 branch August 26, 2022 18:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8290211: jdk/internal/vm/Continuation/Fuzz.java failed with "AssertionError: Failed to compile int Fuzz.com_int(int,int) in 5000ms" #9844

8290211: jdk/internal/vm/Continuation/Fuzz.java failed with "AssertionError: Failed to compile int Fuzz.com_int(int,int) in 5000ms" #9844

dcubed-ojdk commented Aug 11, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Aug 11, 2022

openjdk bot commented Aug 11, 2022

dcubed-ojdk commented Aug 11, 2022

openjdk bot commented Aug 11, 2022

mlbridge bot commented Aug 11, 2022 •

edited

Loading

openjdk bot commented Aug 12, 2022 •

edited

Loading

DamonFool Aug 12, 2022

DamonFool Aug 12, 2022

dcubed-ojdk Aug 12, 2022 •

edited

Loading

dcubed-ojdk left a comment

dcubed-ojdk Aug 12, 2022

dcubed-ojdk Aug 12, 2022 •

edited

Loading

dcubed-ojdk commented Aug 12, 2022

DamonFool commented Aug 12, 2022

DamonFool commented Aug 12, 2022

dcubed-ojdk commented Aug 19, 2022

DamonFool left a comment

dcubed-ojdk commented Aug 20, 2022

dcubed-ojdk commented Aug 22, 2022

openjdk bot commented Aug 22, 2022

openjdk bot commented Aug 22, 2022

dcubed-ojdk commented Aug 22, 2022

8290211: jdk/internal/vm/Continuation/Fuzz.java failed with "AssertionError: Failed to compile int Fuzz.com_int(int,int) in 5000ms" #9844

8290211: jdk/internal/vm/Continuation/Fuzz.java failed with "AssertionError: Failed to compile int Fuzz.com_int(int,int) in 5000ms" #9844

Conversation

dcubed-ojdk commented Aug 11, 2022 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Aug 11, 2022

openjdk bot commented Aug 11, 2022

dcubed-ojdk commented Aug 11, 2022

openjdk bot commented Aug 11, 2022

mlbridge bot commented Aug 11, 2022 • edited Loading

Webrevs

openjdk bot commented Aug 12, 2022 • edited Loading

DamonFool Aug 12, 2022

Choose a reason for hiding this comment

DamonFool Aug 12, 2022

Choose a reason for hiding this comment

dcubed-ojdk Aug 12, 2022 • edited Loading

Choose a reason for hiding this comment

dcubed-ojdk left a comment

Choose a reason for hiding this comment

dcubed-ojdk Aug 12, 2022

Choose a reason for hiding this comment

dcubed-ojdk Aug 12, 2022 • edited Loading

Choose a reason for hiding this comment

dcubed-ojdk commented Aug 12, 2022

DamonFool commented Aug 12, 2022

DamonFool commented Aug 12, 2022

dcubed-ojdk commented Aug 19, 2022

DamonFool left a comment

Choose a reason for hiding this comment

dcubed-ojdk commented Aug 20, 2022

dcubed-ojdk commented Aug 22, 2022

openjdk bot commented Aug 22, 2022

openjdk bot commented Aug 22, 2022

dcubed-ojdk commented Aug 22, 2022

dcubed-ojdk commented Aug 11, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Aug 11, 2022 •

edited

Loading

openjdk bot commented Aug 12, 2022 •

edited

Loading

dcubed-ojdk Aug 12, 2022 •

edited

Loading

dcubed-ojdk Aug 12, 2022 •

edited

Loading