Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Mon, 2 May 2022 17:29:02 GMT, Leonid Mesnik wrote: >> test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp >> line 139: >> >>> 137:thr_info.name,(jni->IsVirtualThread(thread) == >>> JNI_TRUE) ? "virtual" : "kernel", >>> 138: (thr_info.is_daemon == JNI_TRUE) ? "deamon" : "user"); >>> 139: } >> >> I'd suggest to add locals (their names are up to you): >>const char* thr_virtual_tag = jni->IsVirtualThread(thread) == JNI_TRUE ? >> "virtual" : "kernel"; >>const char* thr_daemon_tag == JNI_TRUE) ? "deamon" : "user"; >> >> It is better to place togeter lines: 129+130. >> Line 137 is not aligned, and there are many unneeded spaces. >> The '()' around conditions are not needed. At least, I do not see them >> increasing readability. > > fixed Thanks, I'll mark all the comments related to the JVMTI event tests as resolved. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 06:33:42 GMT, Serguei Spitsyn wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/hotspot/jtreg/serviceability/jvmti/events/ClassPrepare/classprep01/libclassprep01.cpp > line 59: > >> 57: static jvmtiEventCallbacks callbacks; >> 58: static jint result = PASSED; >> 59: static volatile size_t eventsCount = 0; // TODO these 2 vars mofified >> from different threads in getReady/check. What to DO??? > > I'd suggest to use RawMonitorLocker with event_lock or counter_lock to > protect updates of these counters. > You can remove this comment then. The variable eventsCount seems to be used in the sinlge thread only. I looked on the several tests with same variable and similar logic. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 06:43:02 GMT, Serguei Spitsyn wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/hotspot/jtreg/serviceability/jvmti/events/Exception/exception01/libexception01.cpp > line 287: > >> 285: } >> 286: >> 287: eventsCount = 0; > > Counters are not protected with locks. > I'm not sure it is a real problem here but would be better to double-check. The variable eventsCount seems to be used in the sinlge thread only. I looked on the several tests with same variable and similar logic. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 06:09:35 GMT, Serguei Spitsyn wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp > line 139: > >> 137:thr_info.name,(jni->IsVirtualThread(thread) == >> JNI_TRUE) ? "virtual" : "kernel", >> 138: (thr_info.is_daemon == JNI_TRUE) ? "deamon" : "user"); >> 139: } > > I'd suggest to add locals (their names are up to you): >const char* thr_virtual_tag = jni->IsVirtualThread(thread) == JNI_TRUE ? > "virtual" : "kernel"; >const char* thr_daemon_tag == JNI_TRUE) ? "deamon" : "user"; > > It is better to place togeter lines: 129+130. > Line 137 is not aligned, and there are many unneeded spaces. > The '()' around conditions are not needed. At least, I do not see them > increasing readability. fixed - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 05:48:19 GMT, Serguei Spitsyn wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp > line 66: > >> 64: for (i = 0; i < METH_NUM; i++) >> 65: bpEvents[i] = 0; >> 66: } > > Leonid, thank you for converting these tests from nsk.jvmti to provide test > coverage for virtual threads! > As I understand all new tests are C++ based. > So, I'd suggest to always use C++ style of loops: > for (int i = 0; i < METH_NUM; i++) > > This suggestion applies to all new tests in the serviceability/jvmti folder. > One reason to keep old style would be to keep these tests comparable with the > matching nsk.jvmti tests. > However, I tried to compare and found out that the difference is already > pretty big. > You can consider this clean up after integration though. fixed in all ported tests were applicable - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 23:14:45 GMT, Mikhailo Seledtsov wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/lib/jdk/test/lib/thread/VThreadRunner.java line 61: > >> 59: /** >> 60: * Run a task in a virtual thread and wait for it to terminate. >> 61: * If the task completse with an exception then it is thrown by this >> method. > > typo: completse --> completes Thanks, it seems this typo was repeated several times in the test. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 20:57:01 GMT, Erik Gahlin wrote: >> test/jdk/jdk/jfr/api/consumer/TestManyRecordings.java line 57: >> >>> 55: int classLoaderCount = Integer.parseInt(args[0]); >>> 56: int classCount = Integer.parseInt(args[1]); >>> 57: for (int i = 0; i > >> Did you mean classLoaderCount here instead of classCount? Also, please make >> sure there is a space between < and classLoaderCount. > > The JFR "tests" look strange. I would expect a test called TestManyRecording > to start recordings, not create a lot of classes, similar to TestManyClasses. > How is this related to Loom? Could this be a merge gone bad? > > Also, in TestActiveSettingEvent.java > > +settingValues.put(EventNames.VirtualThreadPinned + "#threshold", "20 ms"); > > The reason to exclude a setting (threshold or stackTrace) from a .jfc file is > if it doesn't make sense to configure. For example, if the event is always > instantaneous (so duration is always 0) or periodic (so the stack trace only > show JFR internals) then "threshold" and "stackTrace" can be removed from the > configuration file, but needs to be added to test to avoid false positive. > > The value "20 ms" seems like something a user might want to configure. If the > event is instant, then the value should be "0 ms". It seems that test/jdk/jdk/jfr/api/consumer/TestManyClasses.java, TestManyRecordings.java, and TestParse.java were added for another JFR event (nothing to do with VirtualThreadPinned). @egahlin has contributed some cleanup and these files are removed. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 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'll add the complete set of labels when the PR is further along. >> >> 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I have reviewed the following portions of this change: test/common, test/gtest, test/hotspot/runtime, test/jdk/jfr, test library Feedback: - see several minor comments inline - under runtime/cds/appcds/test-classes there is an empty "TEST.properties" file; was it added by mistake? With only a few minor comments, I approve of the code reviewed by me provided that my comments will be addressed. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 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'll add the complete set of labels when the PR is further along. >> >> 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/lib/jdk/test/lib/thread/VThreadRunner.java line 61: > 59: /** > 60: * Run a task in a virtual thread and wait for it to terminate. > 61: * If the task completse with an exception then it is thrown by this > method. typo: completse --> completes test/lib/jdk/test/lib/thread/VThreadRunner.java line 121: > 119: /** > 120: * Run a task in a virtual thread and wait for it to terminate. > 121: * If the task completse with an exception then it is thrown by this > method. completse --> completes - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 18:27:27 GMT, Mikhailo Seledtsov wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/jdk/jdk/jfr/api/consumer/TestManyRecordings.java line 57: > >> 55: int classLoaderCount = Integer.parseInt(args[0]); >> 56: int classCount = Integer.parseInt(args[1]); >> 57: for (int i = 0; i > Did you mean classLoaderCount here instead of classCount? Also, please make > sure there is a space between < and classLoaderCount. The JFR "tests" look strange. I would expect a test called TestManyRecording to start recordings, not created a lot of classes, similar to TestManyClasses. How is this related to Loom? Could this be a merge gone bad? - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 18:23:35 GMT, Mikhailo Seledtsov wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/jdk/jdk/jfr/api/consumer/TestManyClasses.java line 57: > >> 55: int classLoaderCount = Integer.parseInt(args[0]); >> 56: int classCount = Integer.parseInt(args[1]); >> 57: for (int i = 0; i > Minor: Style: please insert space between < and classCount Also, should this be i < classLoaderCount ?? - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 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'll add the complete set of labels when the PR is further along. >> >> 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/jdk/jdk/jfr/api/consumer/TestManyClasses.java line 57: > 55: int classLoaderCount = Integer.parseInt(args[0]); > 56: int classCount = Integer.parseInt(args[1]); > 57: for (int i = 0; i 62: theClass.newInstance(); > 63: TestEvent event = new TestEvent(); > 64: event.theClass = event; Did you mean event.theClass = theClass instead ? test/jdk/jdk/jfr/api/consumer/TestManyRecordings.java line 57: > 55: int classLoaderCount = Integer.parseInt(args[0]); > 56: int classCount = Integer.parseInt(args[1]); > 57: for (int i = 0; i https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 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'll add the complete set of labels when the PR is further along. >> >> 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/hotspot/jtreg/runtime/vthread/StackChunkClassLoaderTest.java line 2: > 1: /* > 2: * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. Please update copyright year. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I'm sorry for the noise with my comments. I'll continue to discuss it privately unless there is something really important. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
>> JDK version to target.
>>
>> 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'll add the complete set of labels when the PR is further along.
>>
>> 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 Scope 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 incrementally with one additional
> commit since the last revision:
>
> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
test/hotspot/jtreg/serviceability/jvmti/events/ClassPrepare/classprep01/libclassprep01.cpp
line 59:
> 57: static jvmtiEventCallbacks callbacks;
> 58: static jint result = PASSED;
> 59: static volatile size_t eventsCount = 0; // TODO these 2 vars mofified
> from different threads in getReady/check. What to DO???
I'd suggest to use RawMonitorLocker with event_lock or counter_lock to protect
updates of these counters.
You can remove this comment then.
test/hotspot/jtreg/serviceability/jvmti/events/ClassPrepare/classprep01/libclassprep01.cpp
line 201:
> 199: LOG("\n");
> 200:
> 201:
Too many empty lines. It might make sense to do a common cleanup with all edits
like this.
test/hotspot/jtreg/serviceability/jvmti/events/ClassPrepare/classprep01/libclassprep01.cpp
line 258:
> 256: return JNI_VERSION_1_8;
> 257: }
> 258: #endif
Empty line is missed => common cleanup.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/hotspot/jtreg/serviceability/jvmti/events/Exception/exception01/libexception01.cpp line 178: > 176:ex.c_cls, ex.c_name, ex.c_sig, > 177:(jint)(ex.c_loc >> 32), (jint)ex.c_loc); > 178: result = STATUS_FAILED; Unaligned lines in the range: 172-177. There are some non-uinified unneeded spaces at lines 172,175. test/hotspot/jtreg/serviceability/jvmti/events/Exception/exception01/libexception01.cpp line 287: > 285: } > 286: > 287: eventsCount = 0; Counters are not protected with locks. I'm not sure it is a real problem here but would be better to double-check. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/hotspot/jtreg/serviceability/jvmti/events/ClassLoad/classload01/libclassload01.cpp line 139: > 137: primClsEvents[i]++; > 138: LOG( > 139: "TEST FAILED: JVMTI_EVENT_CLASS_LOAD event received for\n" Combine 138+138. I won't comment this more in hope the same will be fixed in all tests. test/hotspot/jtreg/serviceability/jvmti/events/ClassLoad/classload01/libclassload01.cpp line 177: > 175: return JNI_VERSION_1_8; > 176: } > 177: #endif One empty line would be nice to add after 177. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
>> JDK version to target.
>>
>> 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'll add the complete set of labels when the PR is further along.
>>
>> 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 Scope 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 incrementally with one additional
> commit since the last revision:
>
> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 139:
> 137:thr_info.name,(jni->IsVirtualThread(thread) ==
> JNI_TRUE) ? "virtual" : "kernel",
> 138: (thr_info.is_daemon == JNI_TRUE) ? "deamon" : "user");
> 139: }
I'd suggest to add locals (their names are up to you):
const char* thr_virtual_tag = jni->IsVirtualThread(thread) == JNI_TRUE ?
"virtual" : "kernel";
const char* thr_daemon_tag == JNI_TRUE) ? "deamon" : "user";
It is better to place togeter lines: 129+130.
Line 137 is not aligned, and there are many unneeded spaces.
The '()' around conditions are not needed. At least, I do not see them
increasing readability.
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 167:
> 165: result = checkStatus = STATUS_FAILED;
> 166: LOG(
> 167: "TEST FAILED: Breakpoint event with unexpected class
> signature:\n"
Combine lines 166+167.
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 187:
> 185: jboolean isVirtual = jni->IsVirtualThread(thread);
> 186: if (isVirtual != METHODS_ATTRS[i]) {
> 187: LOG("TEST FAILED: IsVirtualThread check failed with unexpected
> result %d when expected is %d\n", isVirtual, METHODS_ATTRS[i]);
Line 187 is too long and can be splitted.
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 231:
> 229: result = STATUS_FAILED;
> 230: LOG(
> 231: "TEST FAILED: wrong number of Breakpoint events\n"
Combine 230+231.
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 307:
> 305: if (agent_lock == NULL) {
> 306: return JNI_ERR;
> 307: }
Better to also use same style with curly brackets in fragments: 293, 296, 299.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp line 66: > 64: for (i = 0; i < METH_NUM; i++) > 65: bpEvents[i] = 0; > 66: } As I understand all new tests are C++ based. So, I'd suggest to always use C++ style of loops: for (int i = 0; i < METH_NUM; i++) - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Marked as reviewed by will-mol...@github.com (no known OpenJDK username). - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Remind: please use the command `/jep JEP-425` [1] to mark this PR. [1] https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/jep - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Marked as reviewed by egahlin (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Thu, 28 Apr 2022 15:10:18 GMT, Markus Grönlund wrote:
>> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java
>> line 35:
>>
>>> 33:
>>> 34: @Category({"Java Runtime"})
>>> 35: @Label("Virtual thread submit task failed")
>>
>> The label is a bit a long, would "Virtual Thread Submit Failed" work?
>
> It works. Thanks.
Yes, this is okay.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Thu, 28 Apr 2022 14:44:05 GMT, Erik Gahlin wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp line 168:
>
>> 166: assert(!tl->is_excluded(), "invariant");
>> 167: if (virtual_thread) {
>> 168: // TODO: blob cache for virtual threads
>
> Fix this now or after integration?
Well spotted. It is an optimization and can happen after integration. I will
create an enhancement for tracking; thank you.
> src/hotspot/share/jfr/metadata/metadata.xml line 121:
>
>> 119: thread="true" stackTrace="true">
>> 120: > description="Thread enlisted as a carrier" />
>> 121: > description="Class of the continuation" />
>
> "Continuation class" = >" Continuation Class"
> numFrames = frames
> numRefs = references
> "Number of interpreted frames" => "Interpreted Frames"
> "Number of references" => "References"
> "Stack size in bytes" => "Stack Size" contentType"bytes"
Will fix.
> src/hotspot/share/jfr/metadata/metadata.xml line 130:
>
>> 128: thread="true" stackTrace="true">
>> 129: > description="Thread enlisted as a carrier" />
>> 130: > description="Class of the continuation" />
>
> contClass => continuationClass
> "Continuation class" => "Continuation Class"
> "Class of the continuation" Remove (not needed)
> "Number of interpreted frames" => "Interpreted Frames"
> numFrames => frames
> "Number of references" => "References"
> numRefs => references
> "Stack size in bytes" => "Stack Size" contentType="bytes"
Will fix.
> src/hotspot/share/jfr/metadata/metadata.xml line 138:
>
>> 136: > category="Java Virtual Machine" label="Continuation Freeze Young"
>> thread="true" stackTrace="false" startTime="false">
>> 137:
>> 138:
>
> "Allocated new" => "Allocated New"
Will fix, thanks.
> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadEndEvent.java line 35:
>
>> 33:
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread end")
>
> "Virtual thread end" => "Virtual Thread End"
> Remove description.
Will fix.
> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadPinnedEvent.java line
> 35:
>
>> 33:
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread pinned")
>
> "Virtual thread pinned" => "Virtual Thread Pinned"
> Remove description
Will fix.
> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadStartEvent.java line 35:
>
>> 33:
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread start")
>
> "virtual thread start" => "Virtual Thread Start"
> Remove description
Will fix, thanks.
> src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java
> line 35:
>
>> 33:
>> 34: @Category({"Java Runtime"})
>> 35: @Label("Virtual thread submit task failed")
>
> The label is a bit a long, would "Virtual Thread Submit Failed" work?
It works. Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Thu, 28 Apr 2022 14:41:04 GMT, Erik Gahlin wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp line 94: > >> 92: static const size_t global_buffer_size = 512 * K; >> 93: >> 94: static const size_t thread_local_buffer_prealloc_count = 32; > > Why is more memory needed? Moore's law or something specific to virtual > threads? The carrier thread will write the metadata for the mounted virtual threads lazily with virtual threads as part of the event write. They write this thread locally, and the memory increase accommodates fewer roundtrips to fetch new buffers. The previous size was small because it would only hold at most one entry. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Thu, 28 Apr 2022 14:37:22 GMT, Erik Gahlin wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp line 81: > >> 79: be_writer.write(size); >> 80: be_writer.write(time); >> 81: be_writer.write(JfrTicks::now().value() - time); > > Why is this changed? It was a small optimization attempted before the current system of writing virtual thread checkpoints in bulk. Will restore it, thank you. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
>> JDK version to target.
>>
>> 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'll add the complete set of labels when the PR is further along.
>>
>> 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 Scope 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 incrementally with one additional
> commit since the last revision:
>
> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp line 168:
> 166: assert(!tl->is_excluded(), "invariant");
> 167: if (virtual_thread) {
> 168: // TODO: blob cache for virtual threads
Fix this now or after integration?
src/hotspot/share/jfr/metadata/metadata.xml line 121:
> 119: thread="true" stackTrace="true">
> 120: description="Thread enlisted as a carrier" />
> 121: description="Class of the continuation" />
"Continuation class" = >" Continuation Class"
numFrames = frames
numRefs = references
"Number of interpreted frames" => "Interpreted Frames"
"Number of references" => "References"
"Stack size in bytes" => "Stack Size" contentType"bytes"
src/hotspot/share/jfr/metadata/metadata.xml line 130:
> 128: thread="true" stackTrace="true">
> 129: description="Thread enlisted as a carrier" />
> 130: description="Class of the continuation" />
contClass => continuationClass
"Continuation class" => "Continuation Class"
"Class of the continuation" Remove (not needed)
"Number of interpreted frames" => "Interpreted Frames"
numFrames => frames
"Number of references" => "References"
numRefs => references
"Stack size in bytes" => "Stack Size" contentType="bytes"
src/hotspot/share/jfr/metadata/metadata.xml line 138:
> 136:category="Java Virtual Machine" label="Continuation Freeze Young"
> thread="true" stackTrace="false" startTime="false">
> 137:
> 138:
"Allocated new" => "Allocated New"
src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp line 94:
> 92: static const size_t global_buffer_size = 512 * K;
> 93:
> 94: static const size_t thread_local_buffer_prealloc_count = 32;
Why is more memory needed? Moore's law or something specific to virtual threads?
src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp line 81:
> 79: be_writer.write(size);
> 80: be_writer.write(time);
> 81: be_writer.write(JfrTicks::now().value() - time);
Why is this changed?
src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadEndEvent.java line 35:
> 33:
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread end")
"Virtual thread end" => "Virtual Thread End"
Remove description.
src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadPinnedEvent.java line 35:
> 33:
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread pinned")
"Virtual thread pinned" => "Virtual Thread Pinned"
Remove description
src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadStartEvent.java line 35:
> 33:
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread start")
"virtual thread start" => "Virtual Thread Start"
Remove description
src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java
line 35:
> 33:
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread submit task failed")
The label is a bit a long, would "Virtual Thread Submit Failed" work?
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 19:43:22 GMT, Paul Sandoz wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/java.base/share/classes/java/util/concurrent/ThreadPerTaskExecutor.java
> line 45:
>
>> 43: * threads is unbounded.
>> 44: */
>> 45: class ThreadPerTaskExecutor implements ExecutorService {
>
> This class manages the set of per-task threads which arguably should be the
> job of the thread container, and it awkwardly overrides the container's set
> of threads by setting a field on `SharedThreadContainer.threadsSupplier`.
>
> `SharedThreadContainer` supports a number of different shared container
> policies that could be made clearer.
>
> Architecturally i think this could be better layered but it can be iterated
> on later.
Indeed, the layering is a bit awkward and I plan to replace it. It came about
as a short term solution to avoid double bookkeeping of virtual threads.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 19:21:58 GMT, Alan Bateman wrote: >> src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java line 122: >> >>> 120: */ >>> 121: >>> 122: public static final int SCOPED_CACHE_SHIFT; >> >> I can't find this constant being used. If added for future, maybe keep >> `UnsafeConstants` class and this static field package-private for now. > > It originally configured the number of lines in extent local cache but the > implementation has changed. @theRealAph would be best to comment on this, it > may be possible to delete it. Yes, it's dead. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Reviewed java.io, java.lang, java.lang.ref. - Marked as reviewed by smarks (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 19:00:52 GMT, Coleen Phillimore wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/hotspot/share/prims/jvmtiEnvBase.cpp line 2388: > >> 2386: } >> 2387: >> 2388: void > > @sspitsyn We should clean up this aberrant style of putting the return type > on the line before the rest of the function declaration after this > integration. It looks so strange. I noticed that it's pre-existing for > other functions in this file. I'm okay to follow any style. We can do it after the integration. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I've reviewed the new files in Hotspot runtime and the cpu directories, as well as the changes to existing files in runtime, oops, utilities, interpreter and classfile. I'm happy to approve this PR. Thank you to everyone for their hard work in reviewing this code, and well done to @pron and @AlanBateman and all the loom team for bringing us this significant and exciting new feature for Java! src/hotspot/share/prims/jvmtiEnvBase.cpp line 2388: > 2386: } > 2387: > 2388: void @sspitsyn We should clean up this aberrant style of putting the return type on the line before the rest of the function declaration after this integration. It looks so strange. I noticed that it's pre-existing for other functions in this file. - Marked as reviewed by coleenp (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e The changes to the `java.io` package and related files in `libjava`, and the changes to the non-networking parts of the `java.nio.channels`, `sun.nio.ch`, and `sun.nio.fs` packages and related files in `libnio` all look fine to me. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e It's great to see end game in play for this multi-year effort, impressive work by all involved. I looked through code in the base module. Generally looks well structured and documented. The fork join code is naturally hard to review. I did try! (it takes the prize for the highest Java code density in the JDK). I mostly looked for regular and repeated structure rather than trying to fully understand/review the intricate concurrency details. IMO there is some post integration work to do around the architecture of thread containers, i presume the structured concurrency implementation (use of thread flock) will help with that. - Marked as reviewed by psandoz (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I reviewed most of the source files in `java.base` except `java.util.concurrent`. I also reviewed `java.logging`, `java.management` and `jdk.management` modules. The changes are easy to follow and clean. Looks good to me. - Marked as reviewed by mchung (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e src/java.base/share/classes/java/lang/Thread.java line 116: > 114: * carrier threads. Locking and I/O operations are examples of > operations > 115: * where a carrier thread may be re-scheduled from one virtual thread to > another. > 116: * Code executing in a virtual thread is not aware of underlying carrier > thread. Suggestion: * Code executing in a virtual thread is not aware of the underlying carrier thread. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
>> JDK version to target.
>>
>> 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'll add the complete set of labels when the PR is further along.
>>
>> 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 Scope 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 incrementally with one additional
> commit since the last revision:
>
> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
src/java.base/share/classes/java/util/concurrent/ThreadPerTaskExecutor.java
line 45:
> 43: * threads is unbounded.
> 44: */
> 45: class ThreadPerTaskExecutor implements ExecutorService {
This class manages the set of per-task threads which arguably should be the job
of the thread container, and it awkwardly overrides the container's set of
threads by setting a field on `SharedThreadContainer.threadsSupplier`.
`SharedThreadContainer` supports a number of different shared container
policies that could be made clearer.
Architecturally i think this could be better layered but it can be iterated on
later.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 18:16:54 GMT, Mandy Chung wrote: >> I was wondering that too, but held off commenting since it's not super >> performance critical given `classToHashes` is synchronized on. However, it >> does make the code a little clearer. > > It's not critical and no problem if this is cleaned up as a follow-up. Good idea, this could be improved. As Paul says, it's not performance critical as this is a diagnostic option for printing the stack trace when pinned. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e The compiler changes look good to me. Marked as reviewed by dlong (Reviewer). - Marked as reviewed by dlong (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 17:53:01 GMT, Mandy Chung wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java line 122:
>
>> 120: */
>> 121:
>> 122: public static final int SCOPED_CACHE_SHIFT;
>
> I can't find this constant being used. If added for future, maybe keep
> `UnsafeConstants` class and this static field package-private for now.
It originally configured the number of lines in extent local cache but the
implementation has changed. @theRealAph would be best to comment on this, it
may be possible to delete it.
> src/java.management/share/classes/sun/management/ThreadImpl.java line 447:
>
>> 445: if (threads != null) {
>> 446: long[] tids = Stream.of(threads)
>> 447: .filter(t -> !(t instanceof
>> jdk.internal.misc.CarrierThread))
>
> Returning an array of thread IDs of just the platform threads is good. The
> javadoc needs to be updated to say "Returns an array of thread identifiers
> for the platform threads"
I think you mean the comment on the private method. Yes, that could be clearer
that it just returns platform threads.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Marked as reviewed by dlong (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 18:13:15 GMT, Paul Sandoz wrote: >> src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58: >> >>> 56: >>> 57: // maps a class to the hashes of stack traces pinned by that code >>> in that class >>> 58: private static final Map, Hashes> classToHashes = new >>> WeakHashMap<>(); >> >> Can you use `ClassValue` in this case? > > I was wondering that too, but held off commenting since it's not super > performance critical given `classToHashes` is synchronized on. However, it > does make the code a little clearer. It's not critical and no problem if this is cleaned up as a follow-up. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 17:27:56 GMT, Mandy Chung wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58: > >> 56: >> 57: // maps a class to the hashes of stack traces pinned by that code in >> that class >> 58: private static final Map, Hashes> classToHashes = new >> WeakHashMap<>(); > > Can you use `ClassValue` in this case? I was wondering that too, but held off commenting since it's not super performance critical given `classToHashes` is synchronized on. However, it does make the code a little clearer. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
>> JDK version to target.
>>
>> 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'll add the complete set of labels when the PR is further along.
>>
>> 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 Scope 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 incrementally with one additional
> commit since the last revision:
>
> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
src/java.management/share/classes/sun/management/ThreadImpl.java line 447:
> 445: if (threads != null) {
> 446: long[] tids = Stream.of(threads)
> 447: .filter(t -> !(t instanceof
> jdk.internal.misc.CarrierThread))
Returning an array of thread IDs of just the platform threads is good. The
javadoc needs to be updated to say "Returns an array of thread identifiers for
the platform threads"
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e src/java.base/share/classes/jdk/internal/misc/UnsafeConstants.java line 122: > 120: */ > 121: > 122: public static final int SCOPED_CACHE_SHIFT; I can't find this constant being used. If added for future, maybe keep `UnsafeConstants` class and this static field package-private for now. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Reviewed debugger-related code (JDI/JDWP/JDB/tests) and partially JVMTI. - Marked as reviewed by amenkov (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e src/java.base/share/classes/java/lang/PinnedThreadPrinter.java line 58: > 56: > 57: // maps a class to the hashes of stack traces pinned by that code in > that class > 58: private static final Map, Hashes> classToHashes = new > WeakHashMap<>(); Can you use `ClassValue` in this case? - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I've reviewed the JVM/TI, JDWP and JDI changes and I'm good with those. I haven't reviewed the JDB changes (forgot about those) and I have not (yet) reviewed the Runtime changes. - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Been through the JDI changes and jdb/example code. Built and manually tested some operations with jdb observing virtual threads. Been through jdk.management/share/classes/com/sun/management and also manually tested jconsole attaching (e.g. dumpThreads operation in both TEXT_PLAIN and JSON). - Marked as reviewed by kevinw (Committer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Reviewed jvmti changes, hotspot tests (excluding tests modified by lmesnik), jdk svc tests changes. - Marked as reviewed by lmesnik (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> 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'll add the complete set of labels when the PR is further along. >> >> 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 Scope 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 incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e Thanks for the refresh Alan! Things look good to me now. I have gone over java.io, the networking parts of java.nio, java.net, and the JMX related changes. For what I have reviewed, I believe this is good to go. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which > JDK version to target. > > 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'll add the complete set of labels when the PR is further along. > > 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 Scope 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 incrementally with one additional commit since the last revision: Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e - Changes: - all: https://git.openjdk.java.net/jdk/pull/8166/files - new: https://git.openjdk.java.net/jdk/pull/8166/files/0864cb09..f2ed4f9c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8166&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8166&range=06-07 Stats: 505 lines in 15 files changed: 139 ins; 189 del; 177 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
