Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 18:54:49 GMT, Alan Bateman wrote: > 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. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 17:43:58 GMT, Aleksey Shipilev wrote: > 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. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 09:35:42 GMT, Alan Bateman wrote: >> 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. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 19 commits: > > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 > - ... and 9 more: > https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec 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. - Changes requested by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 09:35:42 GMT, Alan Bateman wrote: >> 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. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 19 commits: > > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 > - ... and 9 more: > https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
On Thu, 5 May 2022 09:35:42 GMT, Alan Bateman wrote: >> 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. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 19 commits: > > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 > - ... and 9 more: > https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec Studied the java.io package edits, the new JFR events and the new jcmd dump_to_file functionality. Looks good! - Marked as reviewed by coffeys (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v12]
> 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. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 19 commits: - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec - Merge with jdk-19+20 - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 - ... and 9 more: https://git.openjdk.java.net/jdk/compare/1bb4de2e...86bea8ec - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=11 Stats: 99452 lines in 1130 files changed: 91184 ins; 3598 del; 4670 mod Patch: https://git.openjdk.java.net/jdk/pull/8166.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8166/head:pull/8166 PR: https://git.openjdk.java.net/jdk/pull/8166
