8284161: Implementation of Virtual Threads (Preview) #8166

AlanBateman · 2022-04-08T13:43:39Z

This is the implementation of JEP 425: Virtual Threads (Preview).

We will refresh this PR periodically to pick up changes and fixes from the loom repo.

Most of the new mechanisms in the HotSpot VM are disabled by default and require running with --enable-preview to enable.

The patch has support for x64 and aarch64 on the usual operating systems (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for zero and some of the other ports. Additional ports can be contributed via PRs against the fibers branch in the loom repo.

There are changes in many areas. To reduce notifications/mails, the labels have been trimmed down for now to hotspot, serviceability and core-libs. We can add additional labels, if required, as the PR progresses.

The changes include a refresh of java.util.concurrent and ForkJoinPool from Doug Lea's CVS. These changes will probably be proposed and integrated in advance of this PR.

The changes include some non-exposed and low-level infrastructure to support the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to make life a bit easier and avoid having to separate VM changes and juggle branches at this time.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires a CSR request to be approved
Change must be properly reviewed (1 review required, with at least 1 reviewer)

Issues

JDK-8284161: Implementation of Virtual Threads (Preview)
JDK-8284169: Implementation of Virtual Threads (Preview) (CSR)

Reviewers

@farhankhan04 (no known github.com user name / role) ⚠️ Review applies to 21972f45
@furkannzmnn (no known github.com user name / role) ⚠️ Review applies to 21972f45
Lance Andersen (@LanceAndersen - Reviewer) ⚠️ Review applies to 0864cb09
Erik Österlund (@fisk - Reviewer) ⚠️ Review applies to 05781877
Robbin Ehn (@robehn - Reviewer) ⚠️ Review applies to 0864cb09
Serguei Spitsyn (@sspitsyn - Reviewer) ⚠️ Review applies to 0864cb09
Stefan Karlsson (@stefank - Reviewer) ⚠️ Review applies to 0864cb09
Thomas Schatzl (@tschatzl - Reviewer) ⚠️ Review applies to 0864cb09
Daniel Fuchs (@dfuch - Reviewer) ⚠️ Review applies to f2ed4f9c
Leonid Mesnik (@lmesnik - Reviewer) ⚠️ Review applies to f2ed4f9c
Daniel D. Daugherty (@dcubed-ojdk - Reviewer) ⚠️ Review applies to f2ed4f9c
Kevin Walls (@kevinjwalls - Committer) ⚠️ Review applies to f2ed4f9c
Alex Menkov (@alexmenkov - Reviewer) ⚠️ Review applies to f2ed4f9c
Dean Long (@dean-long - Reviewer) ⚠️ Review applies to f2ed4f9c
Mandy Chung (@mlchung - Reviewer) ⚠️ Review applies to f2ed4f9c
Paul Sandoz (@PaulSandoz - Reviewer) ⚠️ Review applies to f2ed4f9c
Brian Burkhalter (@bplb - Reviewer) ⚠️ Review applies to f2ed4f9c
Coleen Phillimore (@coleenp - Reviewer) ⚠️ Review applies to f2ed4f9c
Stuart Marks (@stuart-marks - Reviewer) ⚠️ Review applies to f2ed4f9c
Erik Gahlin (@egahlin - Reviewer) ⚠️ Review applies to f2ed4f9c
@will-molloy (no known github.com user name / role) ⚠️ Review applies to f2ed4f9c
Mikhailo Seledtsov (@mseledts - Committer) ⚠️ Review applies to 51bc652d
Sean Coffey (@coffeys - Reviewer) ⚠️ Review applies to 86bea8ec
Joe Darcy (@jddarcy - Reviewer) ⚠️ Review applies to 86bea8ec

Contributors

Ron Pressler <rpressler@openjdk.org>
Alan Bateman <alanb@openjdk.org>
Erik Österlund <eosterlund@openjdk.org>
Andrew Haley <aph@openjdk.org>
Rickard Bäckman <rbackman@openjdk.org>
Markus Grönlund <mgronlun@openjdk.org>
Leonid Mesnik <lmesnik@openjdk.org>
Serguei Spitsyn <sspitsyn@openjdk.org>
Chris Plummer <cjplummer@openjdk.org>
Coleen Phillimore <coleenp@openjdk.org>
Robbin Ehn <rehn@openjdk.org>
Stefan Karlsson <stefank@openjdk.org>
Thomas Schatzl <tschatzl@openjdk.org>
Sergey Kuksenko <skuksenko@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/8166/head:pull/8166
$ git checkout pull/8166

Update a local copy of the PR:
$ git checkout pull/8166
$ git pull https://git.openjdk.java.net/jdk pull/8166/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8166

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/8166.diff

AlanBateman · 2022-04-08T13:44:21Z

/contributor add rpressler
/contributor add alanb
/contributor add eosterlund
/contributor add aph
/contributor add rbackman
/contributor add mgronlun
/contributor add lmesnik
/contributor add sspitsyn
/contributor add cjplummer
/contributor add coleenp
/contributor add rehn
/contributor add stefank
/contributor add tschatzl
/contributor add skuksenko
/label remove build
/label remove client
/label remove jmx
/label remove kulla
/label remove security
/label remove shenandoah
/label remove nio
/label remove net
/csr

bridgekeeper · 2022-04-08T13:45:11Z

👋 Welcome back alanb! 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-04-08T13:47:19Z

@AlanBateman
Contributor Ron Pressler <rpressler@openjdk.org> successfully added.

openjdk · 2022-04-08T13:48:27Z

@AlanBateman
Contributor Alan Bateman <alanb@openjdk.org> successfully added.

openjdk · 2022-04-08T13:51:46Z

@AlanBateman
Contributor Erik Österlund <eosterlund@openjdk.org> successfully added.

openjdk · 2022-04-08T13:52:59Z

@AlanBateman
Contributor Andrew Haley <aph@openjdk.org> successfully added.

openjdk · 2022-04-08T13:54:13Z

@AlanBateman
Contributor Rickard Bäckman <rbackman@openjdk.org> successfully added.

openjdk · 2022-04-08T13:55:33Z

@AlanBateman
Contributor Markus Grönlund <mgronlun@openjdk.org> successfully added.

openjdk · 2022-04-08T13:56:52Z

@AlanBateman
Contributor Leonid Mesnik <lmesnik@openjdk.org> successfully added.

openjdk · 2022-04-08T13:58:00Z

@AlanBateman
Contributor Serguei Spitsyn <sspitsyn@openjdk.org> successfully added.

openjdk · 2022-04-08T14:00:27Z

@AlanBateman
Contributor Chris Plummer <cjplummer@openjdk.org> successfully added.

openjdk · 2022-04-08T14:01:36Z

@AlanBateman
Contributor Coleen Phillimore <coleenp@openjdk.org> successfully added.

openjdk · 2022-04-08T14:02:46Z

@AlanBateman
Contributor Robbin Ehn <rehn@openjdk.org> successfully added.

openjdk · 2022-04-08T14:03:51Z

@AlanBateman
Contributor Stefan Karlsson <stefank@openjdk.org> successfully added.

openjdk · 2022-04-08T14:05:04Z

@AlanBateman
Contributor Thomas Schatzl <tschatzl@openjdk.org> successfully added.

openjdk · 2022-04-08T14:07:17Z

@AlanBateman
Contributor Sergey Kuksenko <skuksenko@openjdk.org> successfully added.

openjdk · 2022-04-08T14:08:20Z

@AlanBateman The build label was not set.

openjdk · 2022-04-08T14:08:37Z

@AlanBateman The client label was not set.

openjdk · 2022-04-08T14:08:53Z

@AlanBateman The jmx label was not set.

openjdk · 2022-04-08T14:09:07Z

@AlanBateman The kulla label was not set.

openjdk · 2022-04-08T14:09:38Z

@AlanBateman The security label was not set.

openjdk · 2022-04-08T14:09:54Z

@AlanBateman The shenandoah label was not set.

openjdk · 2022-04-08T14:10:10Z

@AlanBateman The nio label was not set.

openjdk · 2022-04-08T14:10:26Z

@AlanBateman The net label was not set.

openjdk · 2022-04-08T14:10:41Z

@AlanBateman an approved CSR request is already required for this pull request.

openjdk · 2022-04-08T14:11:30Z

@AlanBateman The following labels will be automatically applied to this pull request:

build
client
core-libs
hotspot
jmx
kulla
net
nio
security
serviceability
shenandoah

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

shipilev · 2022-05-04T16:02:36Z

The patch has support for x64 and aarch64 on the usual operating systems (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for zero and some of the other ports. Additional ports can be contributed via PRs against the fibers branch in the loom repo.

So, does this PR pass current GHA checks? I see they are not enabled for this PR. It would be unfortunate for this large integration to break builds/tests for smaller PRs that would follow it.

sspitsyn · 2022-05-05T02:22:09Z

I've completed review of new vthread-related tests in the folder serviceability/jvmti.
It includes sub-folders:
serviceability/jvmti/events
serviceability/jvmti/negative
serviceability/jvmti/thread
Thank you, Leonid, for resolving my comments!

AlanBateman · 2022-05-05T09:43:52Z

So, does this PR pass current GHA checks? I see they are not enabled for this PR. It would be unfortunate for this large integration to break builds/tests for smaller PRs that would follow it.

I've enabled it on my fork and we'll see if it kicks in.

coffeys

Studied the java.io package edits, the new JFR events and the new jcmd dump_to_file functionality. Looks good!

openjdk · 2022-05-05T17:43:08Z

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

8284161: Implementation of Virtual Threads (Preview)

Co-authored-by: Ron Pressler <rpressler@openjdk.org>
Co-authored-by: Alan Bateman <alanb@openjdk.org>
Co-authored-by: Erik Österlund <eosterlund@openjdk.org>
Co-authored-by: Andrew Haley <aph@openjdk.org>
Co-authored-by: Rickard Bäckman <rbackman@openjdk.org>
Co-authored-by: Markus Grönlund <mgronlun@openjdk.org>
Co-authored-by: Leonid Mesnik <lmesnik@openjdk.org>
Co-authored-by: Serguei Spitsyn <sspitsyn@openjdk.org>
Co-authored-by: Chris Plummer <cjplummer@openjdk.org>
Co-authored-by: Coleen Phillimore <coleenp@openjdk.org>
Co-authored-by: Robbin Ehn <rehn@openjdk.org>
Co-authored-by: Stefan Karlsson <stefank@openjdk.org>
Co-authored-by: Thomas Schatzl <tschatzl@openjdk.org>
Co-authored-by: Sergey Kuksenko <skuksenko@openjdk.org>
Reviewed-by: lancea, eosterlund, rehn, sspitsyn, stefank, tschatzl, dfuchs, lmesnik, dcubed, kevinw, amenkov, dlong, mchung, psandoz, bpb, coleenp, smarks, egahlin, mseledtsov, coffeys, darcy

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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.

shipilev

I am sorry to be a buzzkill here, but this integration would break lots of platforms even when Loom functionality is not enabled/used. For example, running java -version on RISC-V runs into many issues: TemplateInterpreterGenerator::generate_Continuation_doYield_entry runs into Unimplemented(), UnsafeCopyMemory asserts about UCM table size, NativeDeoptInstruction::is_deopt would run into Unimplemented() while being called from signal handler.

This effectively blocks development on those platforms, and it seems to be too much breakage for a preview feature. Are we really breaking these platforms? Do we have plans in place with maintainers of those platforms to fix all this in JDK 19 timeframe? I'd suggest holding off the integration until such a plan and agreement is in place.

AlanBateman · 2022-05-05T18:54:49Z

I am sorry to be a buzzkill here, but this integration would break lots of platforms even when Loom functionality is not enabled/used. For example, running java -version on RISC-V runs into many issues: TemplateInterpreterGenerator::generate_Continuation_doYield_entry runs into Unimplemented(), UnsafeCopyMemory asserts about UCM table size, NativeDeoptInstruction::is_deopt would run into Unimplemented() while being called from signal handler.

This effectively blocks development on those platforms, and it seems to be too much breakage for a preview feature. Are we really breaking these platforms? Do we have plans in place with maintainers of those platforms to fix all this in JDK 19 timeframe? I'd suggest holding off the integration until such a plan and agreement is in place.

We mailed porters-dev in Sep 2021 to give a heads up that this feature would require porting work so it shouldn't be a surprise. We have been open to including other ports with the initial integration but it was never a goal to have all the ports done in advance of that.

Most of the new code in the VM is only used when running with --enable-preview. You'll see several places that test Continuations::enabled(). It should be possible to get these port running without -enable-preview without much effort. The feature has to be implemented to pass all the tests of course.

shipilev · 2022-05-06T08:29:33Z

We mailed porters-dev in Sep 2021 to give a heads up that this feature would require porting work so it shouldn't be a surprise. We have been open to including other ports with the initial integration but it was never a goal to have all the ports done in advance of that.

It is understandable to ship the preview feature not implemented on all platforms. It is even understandable to ship the final product feature that breaks some platforms in an clearly understandable way, prompting platform maintainers to implement the agreed-upon, final Java feature. What I am seeing, though, is that just running java -version (!) after integrating a preview feature is broken!

Most of the new code in the VM is only used when running with --enable-preview. You'll see several places that test Continuations::enabled(). It should be possible to get these port running without -enable-preview without much effort. The feature has to be implemented to pass all the tests of course.

It is not as clear-cut, unfortunately. I see there are substantial changes in deopt machinery with post-call NOPs -- and there maybe more stuff lurking after that one is implemented -- so it is not as simple as changing Unimplemented() to guarding with Continuations::enabled(). So this looks to me that the core deopt machinery is affected, whether Loom is enabled or not. Is the impact of that deopt machinery change on the VM stability and VM ports discussed, understood, documented somewhere?

I would have honestly expected those core changes to be heavily conditionalized with either ifdef-s, or runtime checks, or both, so that both unimplemented platforms had the old behavior and the implemented platforms had a fallback to old behavior if preview feature was broken. The current code is fine for experimental Loom repo, but I firmly believe mainline should have much stronger safety/reliability requirements.

My fear is that an integration like this would wreck JDK 19 release. So my question stands: Are we breaking those platforms? Are we sure the unconditional VM changes are problem-free, implementable everywhere, etc? The answer might be "Yes, we are integrating, let the chips fall where they may", but I also believe that should be the call made by responsible platform/VM architects, who should explicitly weigh in and accept the risk of wide JDK 19 breakage.

AlanBateman · 2022-05-06T09:47:45Z

It is understandable to ship the preview feature not implemented on all platforms. It is even understandable to ship the final product feature that breaks some platforms in an clearly understandable way, prompting platform maintainers to implement the agreed-upon, final Java feature. What I am seeing, though, is that just running java -version (!) after integrating a preview feature is broken!

Preview features need to be implemented by a port before it can be released. For JDK 19 this means that the maintainers of ports will have work to do for both JEP 424 and JEP 425 (ifdef is not an option).

I think the issue that you are concerned about is the interim period between the JEP 425 integration and the port/implementation of this feature to other architectures. I think the answer is that it will vary. It may be that some port maintainers decide to do something very short term so they can run without --enable-preview and buy time before they do the port/implementation for JDK 19. Good progress has been reported on at least ppc64le port and maybe that port be ready before others. So yes, some ports may crash until they receive attention, others (like zero) should be able to run tests that don't use --enable-preview. I've no doubt there will be a flurry of changes post integration.

The motivation for Continuations::enabled() was to reduce risk and disable a lot of the new code by default. The motivation wasn't porting but it may be helpful during the interim period. That is probably a topic for loom-dev rather than here.

AlanBateman · 2022-05-07T08:04:40Z

/integrate

openjdk · 2022-05-07T08:06:45Z

Going to push as commit 9583e36.

openjdk · 2022-05-07T08:07:04Z

@AlanBateman Pushed as commit 9583e36.

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

Initial push

21972f4

openjdk bot added the csr Pull request needs approved CSR before integration label Apr 8, 2022

openjdk bot added serviceability serviceability-dev@openjdk.org hotspot hotspot-dev@openjdk.org labels Apr 8, 2022

Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a

f41b313

openjdk bot added rfr Pull request is ready for review and removed merge-conflict Pull request has merge conflict with target branch labels May 3, 2022

AlanBateman added 2 commits May 4, 2022 13:02

Merge with 4b2c822

ae9b3f1

Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05

f06aff7

AlanBateman added 2 commits May 5, 2022 09:13

Merge with 1bb4de2

28eb6dc

Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f

86bea8e

coffeys approved these changes May 5, 2022

View reviewed changes

jddarcy approved these changes May 5, 2022

View reviewed changes

openjdk bot removed the csr Pull request needs approved CSR before integration label May 5, 2022

openjdk bot added the ready Pull request is ready to be integrated label May 5, 2022

shipilev suggested changes May 5, 2022

View reviewed changes

AlanBateman added 2 commits May 6, 2022 07:34

Merge with 9425ab2

7c5a1be

Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3

6d96867

AlanBateman added 2 commits May 7, 2022 06:14

Merge with 5212535

d6dd99e

Refresh 4e99b5185eef9398c4cc4e90544de4a3153d61a9

df43e6f

openjdk bot added the integrated Pull request has been integrated label May 7, 2022

openjdk bot closed this May 7, 2022

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

8284161: Implementation of Virtual Threads (Preview) #8166

8284161: Implementation of Virtual Threads (Preview) #8166

Conversation

AlanBateman commented Apr 8, 2022 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Contributors

Reviewing

AlanBateman commented Apr 8, 2022

bridgekeeper bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

openjdk bot commented Apr 8, 2022

shipilev commented May 4, 2022

sspitsyn commented May 5, 2022 • edited Loading

AlanBateman commented May 5, 2022

coffeys left a comment

Choose a reason for hiding this comment

openjdk bot commented May 5, 2022 • edited Loading

shipilev left a comment

Choose a reason for hiding this comment

AlanBateman commented May 5, 2022

shipilev commented May 6, 2022 • edited Loading

AlanBateman commented May 6, 2022 • edited Loading

AlanBateman commented May 7, 2022

openjdk bot commented May 7, 2022

openjdk bot commented May 7, 2022

AlanBateman commented Apr 8, 2022 •

edited by openjdk bot

Loading

sspitsyn commented May 5, 2022 •

edited

Loading

openjdk bot commented May 5, 2022 •

edited

Loading

shipilev commented May 6, 2022 •

edited

Loading

AlanBateman commented May 6, 2022 •

edited

Loading