Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.
/ jdk20u Public archive

8300575: JVMTI support when using alternative virtual thread implementation #60

Closed
wants to merge 1 commit into from

Conversation

backwaterred
Copy link

@backwaterred backwaterred commented Apr 24, 2023

This PR backports modifications to the jvmti code to add support for vthreads. Without this change, we see many failures in our internal testing.

These changes did not apply cleanly in several places because of changes related to JDK-8299837 (Replace NULL with nullptr in hotspot code and tests). In these places, the changes expected code containing nullptr, but found NULL. In these spots, kept the code from the backported change, but changed nullptr back to NULL for consistency with the rest of the code base.

Testing is underway.


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-8300575: JVMTI support when using alternative virtual thread implementation

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 60

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk20u/pull/60.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Apr 24, 2023

👋 Welcome back tsteele! 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 openjdk bot changed the title Backport 83bea26df453282d46afff333e006e8f9b7fb201 8300575: JVMTI support when using alternative virtual thread implementation Apr 24, 2023
@openjdk
Copy link

openjdk bot commented Apr 24, 2023

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added backport rfr Pull request is ready for review labels Apr 24, 2023
@mlbridge
Copy link

mlbridge bot commented Apr 24, 2023

Webrevs

@AlanBateman
Copy link

Are you sure this is needed JVMTI is optional and a build with the alt implementation doesn't support JVMTI with --enable-preview. So I wouldn't expect you should be running the JVMTI conformance tests with --enable-preview.

@backwaterred
Copy link
Author

Things get a bit cryptic when discussing the conformance tests, so feel free to reach out via email if you find my response is not providing the detail you're looking for.

Are you sure this is needed[?] JVMTI is optional...

Basically, yes I'm sure. And, we've received guidance on one of the mailing lists that backporting this change is a possible solution to the difficulty we are facing.

... and a build with the alt implementation doesn't support JVMTI with --enable-preview.

Unless I've misunderstood your comment. Yes, but the changes I'm backporting are meant to enable it.

…tation

Reviewed-by: lmesnik, sspitsyn, alanb
@backwaterred backwaterred marked this pull request as draft April 24, 2023 18:17
@backwaterred backwaterred marked this pull request as ready for review April 24, 2023 18:17
@AlanBateman
Copy link

AlanBateman commented Apr 24, 2023

Things get a bit cryptic when discussing the conformance tests, so feel free to reach out via email if you find my response is not providing the detail you're looking for.

Are you sure this is needed[?] JVMTI is optional...

Basically, yes I'm sure. And, we've received guidance on one of the mailing lists that backporting this change is a possible solution to the difficulty we are facing.

... and a build with the alt implementation doesn't support JVMTI with --enable-preview.

Unless I've misunderstood your comment. Yes, but the changes I'm backporting are meant to enable it.

The changes in JDK-8300575 are not needed in JDK 20u. Not objection to backporting the change but my guess is there is some confusion somewhere that an implementation is required to support JVMTI. JVMTI has always been optional. In this case, JVMTI is not supported in JDK 19/20 when using the alternative implementation of virtual threads and when running with --enable-preview.

@bridgekeeper
Copy link

bridgekeeper bot commented May 29, 2023

@backwaterred This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport rfr Pull request is ready for review
2 participants