8301306: java/net/httpclient/* fail with -Xcomp #12282

sunny868 · 2023-01-30T07:16:17Z

Hi all,
I think BackgroundCompilation should not be set to false when -Xcomp be used, which causes the java thread to block for longer, then causing the following tests failed frequently on the AArch64 and LoongArch64 architecture.

java/net/vthread/HttpALot.java (release -Xcomp)
java/net/httpclient/ByteArrayPublishers.java （fastdebug -Xcomp）
java/net/httpclient/ManyRequestsLegacy.java （fastdebug -Xcomp）
java/net/httpclient/HttpClientLocalAddrTest.java （fastdebug -Xcomp）
java/net/httpclient/SpecialHeadersTest.java (fastdebug -Xcomp）

This PR fix the issue, Please help review it.

Thanks.

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-8301306: java/net/httpclient/* fail with -Xcomp

Reviewers

Daniel Fuchs (@dfuch - Reviewer)
Alan Bateman (@AlanBateman - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 12282

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

Using diff file

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

…ion: HTTP/1.1 header parser received no bytes

bridgekeeper · 2023-01-30T07:17:46Z

👋 Welcome back sunny868! 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 · 2023-01-30T07:20:07Z

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

hotspot-runtime

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 · 2023-01-30T07:23:59Z

Webrevs

dcubed-ojdk · 2023-01-30T21:10:31Z

/label core-libs

openjdk · 2023-01-30T21:11:53Z

@dcubed-ojdk
The core-libs label was successfully added.

dcubed-ojdk

This is not the right fix for this kind of problem.

dcubed-ojdk · 2023-01-30T21:13:12Z

src/hotspot/share/runtime/arguments.cpp

@@ -1437,7 +1437,6 @@ void Arguments::set_mode_flags(Mode mode) {
    break;
  case _comp:
    UseInterpreter           = false;
-    BackgroundCompilation    = false;


Disabling BackgroundCompilation when -Xcomp is specified is not the right
answer here. When -Xcomp is specified, -Xbatch needs to be turned off
and I believe that's what old L1440 is doing.

Dan is right, you can't change the definition of -Xcomp.

But -Xbatch is -XX:-BackgroundCompilation. So the second Dan's statement is incorrect.

dcubed-ojdk · 2023-01-30T21:15:53Z

/label add hotspot-compiler

openjdk · 2023-01-30T21:16:45Z

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

AlanBateman · 2023-01-31T07:17:59Z

There was some discussion about these test failures in the JBS issue and I think @sunny868 has accepted that they need to be investigated to see if they are test reliability issues or something else. As always, running tests with -Xcomp can often challenge timeouts and assumption of progress in tests.

vnkozlov · 2023-01-31T17:07:53Z

There was some discussion about these test failures in the JBS issue and I think @sunny868 has accepted that they need to be investigated to see if they are test reliability issues or something else. As always, running tests with -Xcomp can often challenge timeouts and assumption of progress in tests.

Yes, -Xcomp can cause long time execution since it we compile all executed methods. But it should not affect execution unless Java code depends on time itself which it should not do.

AlanBateman · 2023-02-02T12:18:17Z

/label add net
/label remove core-libs
/label remove hotspot-runtime
/label remove hotspot-compiler

openjdk · 2023-02-02T12:19:12Z

@AlanBateman
The net label was successfully added.

openjdk · 2023-02-02T12:20:34Z

@AlanBateman
The core-libs label was successfully removed.

openjdk · 2023-02-02T12:21:48Z

@AlanBateman
The hotspot-runtime label was successfully removed.

openjdk · 2023-02-02T12:23:19Z

@AlanBateman
The hotspot-compiler label was successfully removed.

AlanBateman · 2023-02-02T12:29:31Z

test/jdk/java/net/vthread/HttpALot.java

@@ -35,7 +35,7 @@
 * @run main/othervm/timeout=600
 *     -Dsun.net.httpserver.nodelay=true
 *     -Dsun.net.client.defaultConnectTimeout=5000
- *     -Dsun.net.client.defaultReadTimeout=5000
+ *     -Dsun.net.client.defaultReadTimeout=50000


For this one, it might be simpler to just drop the setting of the defaultReadTimeout.

OK, I'll test it on my local machine later

sunny868 · 2023-02-02T12:51:29Z

The following two tests have the same issue, can I submit the fix code to the same patch?
java/rmi/server/UnicastRemoteObject/serialFilter/FilterUROTest.java and jdk/incubator/concurrent/StructuredTaskScope/StructuredTaskScopeTest.java

AlanBateman · 2023-02-02T12:55:09Z

The following two tests have the same issue, can I submit the fix code to the same patch?
java/rmi/server/UnicastRemoteObject/serialFilter/FilterUROTest.java and jdk/incubator/concurrent/StructuredTaskScope/StructuredTaskScopeTest.java

StructuredTaskScopeTest runs 46 tests, and runs them for both platform and virtual threads. Which test is failing? There is no information in the JBS issue on this.

dfuch

The proposed changes look reasonable to me.

sunny868 · 2023-02-03T01:15:06Z

The following two tests have the same issue, can I submit the fix code to the same patch?
java/rmi/server/UnicastRemoteObject/serialFilter/FilterUROTest.java and jdk/incubator/concurrent/StructuredTaskScope/StructuredTaskScopeTest.java

StructuredTaskScopeTest runs 46 tests, and runs them for both platform and virtual threads. Which test is failing? There is no information in the JBS issue on this.

test StructuredTaskScopeTest.testInterruptJoinUntil1(java.lang.ThreadBuilders$PlatformThreadFactory@6e8e0c75): failure
I had set up JDK-8301736 and JDK-8301737 to fix these two test cases.

sunny868 · 2023-02-03T06:20:30Z

RFR, thanks

AlanBateman

Thanks for the update and creating JBS issues/PRs for the -Xcomp vs. timeout issues in tests in other areas.

openjdk · 2023-02-03T07:28:01Z

⚠️ @sunny868 the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout 8301306
$ git commit --author='Preferred Full Name <you@example.com>' --allow-empty -m 'Update full name'
$ git push

openjdk · 2023-02-03T07:28:03Z

@sunny868 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:

8301306: java/net/httpclient/* fail with -Xcomp

Reviewed-by: dfuchs, alanb

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 101 new commits pushed to the master branch:

cf68d9f: 8299155: C2: SubTypeCheckNode::verify() should not produce dependencies / oop pool entries
11804b2: 8301050: Detect Xen Virtualization on Linux aarch64
b504c94: 8300235: Use VarHandle access in Image(Input | Output)StreamImpl classes
406021a: 8300929: Avoid unnecessary array fill after creation in java.awt.image
7f313b0: 8180266: Convert sun/security/provider/KeyStore/DKSTest.sh to Java Jtreg Test
3ad6aef: 8301313: RISC-V: C2: assert(false) failed: bad AD file due to missing match rule
4c9de87: 8301655: Problemlist jdk/jdk/nio/zipfs/TestLocOffsetFromZip64EF.java on Linux
04278e6: 8301564: Non-C-heap allocated ResourceHashtable keys and values must have trivial destructor
b00b70c: 8286907: keytool should warn about weak PBE algorithms
ee0f5b5: 8301392: Port fdlibm log1p to Java
... and 91 more: https://git.openjdk.org/jdk/compare/64b25ea0b410542635b6d99a92ec290da47c85ce...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@dcubed-ojdk, @dfuch, @AlanBateman) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

sunny868 · 2023-02-03T07:29:06Z

/integrate

openjdk · 2023-02-03T07:30:30Z

@sunny868
Your change (at version 215b5b7) is now ready to be sponsored by a Committer.

sunny868 · 2023-02-03T07:56:45Z

@AlanBateman Could you please sponsor it for me?

dfuch

LGTM

DamonFool · 2023-02-03T11:54:31Z

/sponsor

openjdk · 2023-02-03T11:56:12Z

Going to push as commit 6f9106e.
Since your change was applied there have been 102 commits pushed to the master branch:

ac9e046: 8301479: Replace NULL with nullptr in os/linux
cf68d9f: 8299155: C2: SubTypeCheckNode::verify() should not produce dependencies / oop pool entries
11804b2: 8301050: Detect Xen Virtualization on Linux aarch64
b504c94: 8300235: Use VarHandle access in Image(Input | Output)StreamImpl classes
406021a: 8300929: Avoid unnecessary array fill after creation in java.awt.image
7f313b0: 8180266: Convert sun/security/provider/KeyStore/DKSTest.sh to Java Jtreg Test
3ad6aef: 8301313: RISC-V: C2: assert(false) failed: bad AD file due to missing match rule
4c9de87: 8301655: Problemlist jdk/jdk/nio/zipfs/TestLocOffsetFromZip64EF.java on Linux
04278e6: 8301564: Non-C-heap allocated ResourceHashtable keys and values must have trivial destructor
b00b70c: 8286907: keytool should warn about weak PBE algorithms
... and 92 more: https://git.openjdk.org/jdk/compare/64b25ea0b410542635b6d99a92ec290da47c85ce...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-02-03T11:56:25Z

@DamonFool @sunny868 Pushed as commit 6f9106e.

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

dfuch · 2023-02-03T15:01:20Z

test/jdk/java/net/httpclient/SpecialHeadersTest.java

@@ -30,6 +30,7 @@
 * @build jdk.httpclient.test.lib.common.HttpServerAdapters
 *        jdk.httpclient.test.lib.http2.Http2TestServer
 *        jdk.test.lib.net.SimpleSSLContext
+ * @requests (vm.compMode != "Xcomp")


I didn't notice this change while reviewing - it is causing trouble in the CI.

Is PR #12399 related?

Looks like it - is that RMI test in #12399 using an HttpServer?

It seems that here @requests should have been @requires ; proposed fix #12409

WRT #12399: FilterUROTest does NOT use an HttpServer.
It runs as "testng/othervm" so perhaps if jtreg uses an HttpServer there may be a connection.

I doubt that what is proposed in #12399 is the right fix for the issue then. I commented there. Still looking for a reviewer for #12409 BTW ;-)

I'm sorry for my low-level typo ;-(

JDK-8301306: java/net/httpclient/* fail with -Xcomp: java.io.IOExcept…

b193aca

…ion: HTTP/1.1 header parser received no bytes

openjdk bot added the rfr Pull request is ready for review label Jan 30, 2023

openjdk bot added the hotspot-runtime hotspot-runtime-dev@openjdk.org label Jan 30, 2023

sunny868 changed the title ~~JDK-8301306: java/net/httpclient/* fail with -Xcomp: java.io.IOException: HTTP/1.1 header parser received no bytes~~ 8301306: java/net/httpclient/* fail with -Xcomp: java.io.IOException: HTTP/1.1 header parser received no bytes Jan 30, 2023

openjdk bot added the core-libs core-libs-dev@openjdk.org label Jan 30, 2023

dcubed-ojdk suggested changes Jan 30, 2023

View reviewed changes

openjdk bot added the hotspot-compiler hotspot-compiler-dev@openjdk.org label Jan 30, 2023

8301306: java/net/httpclient/* fail with -Xcomp

449ff65

sunny868 changed the title ~~8301306: java/net/httpclient/* fail with -Xcomp: java.io.IOException: HTTP/1.1 header parser received no bytes~~ 8301306: java/net/httpclient/* fail with -Xcomp Feb 2, 2023

openjdk bot changed the title ~~8301306: java/net/httpclient/* fail with -Xcomp~~ 8301306: java/net/httpclient/* fail with -Xcomp: java.io.IOException: HTTP/1.1 header parser received no bytes Feb 2, 2023

openjdk bot added the net net-dev@openjdk.org label Feb 2, 2023

openjdk bot removed the core-libs core-libs-dev@openjdk.org label Feb 2, 2023

openjdk bot removed the hotspot-runtime hotspot-runtime-dev@openjdk.org label Feb 2, 2023

openjdk bot removed the hotspot-compiler hotspot-compiler-dev@openjdk.org label Feb 2, 2023

AlanBateman reviewed Feb 2, 2023

View reviewed changes

8301306: java/net/httpclient/* fail with -Xcomp

a99f097

dfuch reviewed Feb 2, 2023

View reviewed changes

sunny868 closed this Feb 3, 2023

sunny868 reopened this Feb 3, 2023

8301306: java/net/httpclient/* fail with -Xcomp

215b5b7

sunny868 changed the title ~~8301306: java/net/httpclient/* fail with -Xcomp: java.io.IOException: HTTP/1.1 header parser received no bytes~~ 8301306: java/net/httpclient/* fail with -Xcomp Feb 3, 2023

AlanBateman approved these changes Feb 3, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 3, 2023

openjdk bot added the sponsor Pull request is ready to be sponsored label Feb 3, 2023

dfuch approved these changes Feb 3, 2023

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Feb 3, 2023

openjdk bot closed this Feb 3, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Feb 3, 2023

dfuch reviewed Feb 3, 2023

View reviewed changes

8301306: java/net/httpclient/* fail with -Xcomp #12282

8301306: java/net/httpclient/* fail with -Xcomp #12282

Conversation

sunny868 commented Jan 30, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Jan 30, 2023

openjdk bot commented Jan 30, 2023

mlbridge bot commented Jan 30, 2023 • edited Loading

Webrevs

dcubed-ojdk commented Jan 30, 2023

openjdk bot commented Jan 30, 2023

dcubed-ojdk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcubed-ojdk commented Jan 30, 2023

openjdk bot commented Jan 30, 2023

AlanBateman commented Jan 31, 2023

vnkozlov commented Jan 31, 2023

AlanBateman commented Feb 2, 2023

openjdk bot commented Feb 2, 2023

openjdk bot commented Feb 2, 2023

openjdk bot commented Feb 2, 2023

openjdk bot commented Feb 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunny868 commented Feb 2, 2023

AlanBateman commented Feb 2, 2023

dfuch left a comment

Choose a reason for hiding this comment

sunny868 commented Feb 3, 2023 • edited Loading

sunny868 commented Feb 3, 2023

AlanBateman left a comment

Choose a reason for hiding this comment

openjdk bot commented Feb 3, 2023

openjdk bot commented Feb 3, 2023 • edited Loading

sunny868 commented Feb 3, 2023

openjdk bot commented Feb 3, 2023

sunny868 commented Feb 3, 2023

dfuch left a comment

Choose a reason for hiding this comment

DamonFool commented Feb 3, 2023

openjdk bot commented Feb 3, 2023

openjdk bot commented Feb 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dfuch Feb 3, 2023 • edited Loading

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

sunny868 commented Jan 30, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Jan 30, 2023 •

edited

Loading

sunny868 commented Feb 3, 2023 •

edited

Loading

openjdk bot commented Feb 3, 2023 •

edited

Loading

dfuch Feb 3, 2023 •

edited

Loading