Integrated: 8288214: serviceability/jvmti/vthread/VThreadNotifyFramePopTest/VThreadNotifyFramePopTest.java test failed
On Sat, 11 Jun 2022 08:11:34 GMT, Alan Bateman wrote: > This test connects to http://openjdk.java.net/ so it's not reliable if the > host name can't be resolved or a HTTP connection cannot be established. I've > changed the test to use a local HTTP server so the original test works as > before, it's just a local rather than remote HTTP connection. > > I did a few cleanups to the test while I was there: > - changed the test to use `@enablePreview`. > - renamed "VThread" in the comment to "virtual thread". > - changed the test to use Executors.newVirtualThreadPerTaskExecutor to avoid > needing to specify a ThreadFactory. > - add a call Future::get so that if the virtual thread terminates with an > exception then the test will fail quickly, avoids needing to calling into the > native/agent code to check if it passed. This pull request has now been integrated. Changeset: c76a06ae Author:Alan Bateman URL: https://git.openjdk.org/jdk19/commit/c76a06aeb5fe7d7630736a74aad8c873b7afe36b Stats: 58 lines in 2 files changed: 39 ins; 4 del; 15 mod 8288214: serviceability/jvmti/vthread/VThreadNotifyFramePopTest/VThreadNotifyFramePopTest.java test failed Reviewed-by: lmesnik, zgu, dcubed, sspitsyn - PR: https://git.openjdk.org/jdk19/pull/6
Re: RFR: 8288324: Loom: Uninitialized JvmtiEnvs in VM_Virtual* ops
On Tue, 14 Jun 2022 07:42:08 GMT, David Holmes wrote: > I'm confused by the comments above. The code failed to initialize the `_env` > member but then makes calls via this uninitialized pointer! Surely we should > have crashed? JvmtiEnvBase::get_frame_count is static and I think `((JvmtiEnvBase*)_env)->get_frame_count(_vthread_h(), _count_ptr);` is compiled to just call it and the not-initialised _env is not used. - PR: https://git.openjdk.org/jdk19/pull/10
Re: RFR: 8288324: Loom: Uninitialized JvmtiEnvs in VM_Virtual* ops
On Mon, 13 Jun 2022 18:14:56 GMT, Aleksey Shipilev wrote: > SonarCloud reports a few uninitialized fields in new VM_Virtual* ops. Those > fields are used, and therefore this is a serious bug. These ops seem to be > used only from a few corner cases, which is probably why this was never > actually found in testing. > > Additional testing: > - [x] Linux x86_64 fastdebug, `jdk_loom hotspot_loom` > - [x] Linux x86_64 fastdebug, `serviceability/jvmti` @sspitsyn @lmesnik If I read this correctly then this calling JVMTI GetFrameCount or GetStackTrace with a jthread to a virtual thread that is not mounted. I'm surprised this wasn't caught by any tests. Should we add tests for these cases? - PR: https://git.openjdk.org/jdk19/pull/10
RFR: 8286176: Add JNI_VERSION_19 to jni.h and JNI spec
JNI is updated in Java 19 so we need to define JNI_VERSION_19 and change GetVersion to return this version. test/hotspot/jtreg/native_sanity/JniVersion.java is updated to check that JNI_VERSION_19 is returned. The native library in the JMX agent, and several tests, define JNI_OnLoad that return JNI_VERSION_10 as the JNI version needed. These are updated to return JNI_VERSION_19 although these updates aren't strictly needed. - Commit messages: - Initial commit Changes: https://git.openjdk.org/jdk19/pull/7/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk19=7=00 Issue: https://bugs.openjdk.org/browse/JDK-8286176 Stats: 16 lines in 10 files changed: 2 ins; 0 del; 14 mod Patch: https://git.openjdk.org/jdk19/pull/7.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/7/head:pull/7 PR: https://git.openjdk.org/jdk19/pull/7
RFR: 8288214: serviceability/jvmti/vthread/VThreadNotifyFramePopTest/VThreadNotifyFramePopTest.java test failed
This test connects to http://openjdk.java.net/ so it's not reliable if the host name can't be resolved or a HTTP connection cannot be established. I've changed the test to use a local HTTP server so the original test works as before, it's just a local rather than remote HTTP connection. I did a few cleanups to the test while I was there: - changed the test to use `@enablePreview`. - renamed "VThread" in the comment to "virtual thread". - changed the test to use Executors.newVirtualThreadPerTaskExecutor to avoid needing to specify a ThreadFactory. - add a call Future::get so that if the virtual thread terminates with an exception then the test will fail quickly, avoids needing to calling into the native/agent code to check if it passed. - Commit messages: - Merge - Initial commit Changes: https://git.openjdk.org/jdk19/pull/6/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk19=6=00 Issue: https://bugs.openjdk.org/browse/JDK-8288214 Stats: 57 lines in 2 files changed: 38 ins; 4 del; 15 mod Patch: https://git.openjdk.org/jdk19/pull/6.diff Fetch: git fetch https://git.openjdk.org/jdk19 pull/6/head:pull/6 PR: https://git.openjdk.org/jdk19/pull/6
Re: RFR: 8288222: ProblemList serviceability/jvmti/vthread/VThreadNotifyFramePopTest/VThreadNotifyFramePopTest.java
On Fri, 10 Jun 2022 15:21:34 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList > serviceability/jvmti/vthread/VThreadNotifyFramePopTest/VThreadNotifyFramePopTest.java. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/4
Re: RFR: 8286983: rename jdb -trackvthreads and debug agent enumeratevthreads options and clarify "Preview Feature" nature of these options [v5]
On Tue, 31 May 2022 18:14:51 GMT, Chris Plummer wrote: >> As part of the loom integration, jdb added `-trackvthreads` and the debug >> agent added `enumeratevthreads`. These options are being renamed to >> `-trackallthreads` and `includevirtualthreads` (the shorthand `vthreads` >> should not have been used). Also, the help text for these options now calls >> out that virtual threads are a Preview Feature. >> >> After the update to help text, wlil look as follows: >> >> jdb doc (search for "trackallthreads"): >> http://cr.openjdk.java.net/~cjplummer/8286983/jdb.html >> debug agent doc (seach for "includevirtualthreads"): >> http://cr.openjdk.java.net/~cjplummer/8286983/conninv.html >> >> >> $ jdb -listconnectors >> ... >> Connector: com.sun.jdi.CommandLineLaunch Transport: dt_socket >> description: Launches target using Sun Java VM command line and attaches >> to it >> ... >> Argument: includevirtualthreads Default value: n >> description: List of all threads includes virtual threads as well as >> platform threads. Virtual threads are a preview feature of the Java platform. >> >> >> >> $ jdb -help >> Usage: jdb >> >> where options include: >> ... >> -dbgtrace [flags] print info for debugging jdb >> -trackallthreads Track all threads, including virtual threads. >> Virtual threads are a preview feature of the Java >> platform. >> -tclient run the application in the HotSpot(TM) Client Compiler >> ... >> >> >> >> $ man -M ./build/linux-x64-debug/images/jdk/man/ jdb >> ... >>-tclient >> Runs the application in the Java HotSpot VM client. >> >>-trackallthreads >> Track all threads as they are created, including >> Virtual >> Threads. See Working With Virtual Threads below. >> Virtual >> threads are a preview feature of the Java platform. >> >>-tserver >> Runs the application in the Java HotSpot VM server. >> ... >> WORKING WITH VIRTUAL THREADS >>Virtual threads are a preview feature of the Java platform. >> Preview >>features may be removed in a future release, or upgraded to >> permanent >>features of the Java platform. >> >>Often virtual theads are created in such large numbers and >> frequency >> ... >> >> >> >> $ java -agentlib:jdwp=help >>Java Debugger JDWP Agent Library >> >> >> (See the "VM Invocation Options" section of the JPDA >>"Connection and Invocation Details" document for more information.) >> >> jdwp usage: java -agentlib:jdwp=[help]|[=, ...] >> >> Option Name and ValueDescription Default >> ---- --- >> ... >> timeout= for listen/attach in milliseconds n >> includevirtualthreads=y|nList of all threads includes virtual >> threads as well as platform threads. >> Virtual threads are a preview feature of >> the Java platform. >>n >> mutf8=y|noutput modified utf-8 n >> ... > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Don't capitalize "virtual threads" Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8780
Re: RFR: 8287726: Fix JVMTI tests with "requires vm.continuations" after JDK-8287496
On Thu, 2 Jun 2022 09:54:31 GMT, Aleksey Shipilev wrote: > [JDK-8287496](https://bugs.openjdk.java.net/browse/JDK-8287496) brought the > alternative Loom implementation that can be used by ports as the fallback. > That fallback does not support JVMTI entirely, so lots of tests fail. Some > JVMTI is still supported, so cutting off at `@requires vm.jvmti` seems too > broad. They should be predicated with `@requires vm.continuations` to be > skipped when fallback is used. > > This also allows reverting relevant x86_32 problemlist exclusions. > > Additional testing: > - [x] Linux x86_32 fastdebug, `serviceability/jvmti` works, many tests > skipped > - [x] Linux x86_64 fastdebug, `serviceability/jvmti` still works, with all > current tests executing Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8990
Re: RFR: 8287726: Fix JVMTI tests with "requires vm.continuations" after JDK-8287496
On Thu, 2 Jun 2022 09:54:31 GMT, Aleksey Shipilev wrote: > [JDK-8287496](https://bugs.openjdk.java.net/browse/JDK-8287496) brought the > alternative Loom implementation that can be used by ports as the fallback. > That fallback does not support JVMTI entirely, so lots of tests fail. Some > JVMTI is still supported, so cutting off at `@requires vm.jvmti` seems too > broad. They should be predicated with `@requires vm.continuations` to be > skipped when fallback is used. > > This also allows reverting relevant x86_32 problemlist exclusions. > > Additional testing: > - [x] Linux x86_32 fastdebug, `serviceability/jvmti` works, many tests > skipped > - [x] Linux x86_64 fastdebug, `serviceability/jvmti` still works, with all > current tests executing I expect you can unexclude the runtime/* tests from this section too. Also the same section in test/jdk/ProblemList.txt that excludes the tests on x86_32 can be cleaned up too, maybe a separate PR. - PR: https://git.openjdk.java.net/jdk/pull/8990
Integrated: 8287496: Alternative virtual thread implementation that maps to OS thread
On Sun, 29 May 2022 14:46:39 GMT, Alan Bateman wrote: > This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. This pull request has now been integrated. Changeset: 6ff2d89e Author:Alan Bateman URL: https://git.openjdk.java.net/jdk/commit/6ff2d89ea11934bb13c8a419e7bad4fd40f76759 Stats: 747 lines in 72 files changed: 574 ins; 53 del; 120 mod 8287496: Alternative virtual thread implementation that maps to OS thread Reviewed-by: rehn, mchung - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
On Wed, 1 Jun 2022 06:26:23 GMT, David Holmes wrote: >> Alan Bateman has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 11 commits: >> >> - Fixed another typo in comment >> - Merge >> - Fix typos in comments >> - Allowing linking without intrinsic stub, contributed-by: rehn >> - Continuation clinit should fail if no continuations support >> - Fix merge issue with test >> - Change VMContinuations to be experimental option, ensure JNI GetEnv >> returns EVERSION >> - Update >> - Expend test coverage >> - Merge >> - ... and 1 more: >> https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6 > > src/java.base/share/classes/java/lang/Thread.java line 742: > >> 740: * @param name thread name, can be null >> 741: * @param characteristics thread characteristics >> 742: * @param bound true when bound to an OS thread > > Nit: s/OS/VM/ ? I think it would be confusing to use "VM" here. The intention is that "true" means the virtual thread will be bound to an OS thread once it has started. Yes, technically it's whatever the VM implements but I think that is too much to try to explain in the parameter description. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
> This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits: - Fixed another typo in comment - Merge - Fix typos in comments - Allowing linking without intrinsic stub, contributed-by: rehn - Continuation clinit should fail if no continuations support - Fix merge issue with test - Change VMContinuations to be experimental option, ensure JNI GetEnv returns EVERSION - Update - Expend test coverage - Merge - ... and 1 more: https://git.openjdk.java.net/jdk/compare/3deb58a8...a5c06cb6 - Changes: https://git.openjdk.java.net/jdk/pull/8939/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8939=02 Stats: 747 lines in 72 files changed: 574 ins; 53 del; 120 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
On Tue, 31 May 2022 17:03:54 GMT, Aleksey Shipilev wrote: > I expected this change to fix the broken ARM32 port, but it doesn't work. There is work required to get the arm32 port working again, currently tracked as JDK-828636 but there may be further issues beyond that. - PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v2]
> This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Allowing linking without intrinsic stub, contributed-by: rehn - Changes: - all: https://git.openjdk.java.net/jdk/pull/8939/files - new: https://git.openjdk.java.net/jdk/pull/8939/files/7989708b..126e38b6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8939=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8939=00-01 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
On Sun, 29 May 2022 14:46:39 GMT, Alan Bateman wrote: > This patch adds an alternative virtual thread implementation where each > virtual thread is backed by an OS thread. It doesn't scale but it can be used > by ports that don't have continuations support in the VM. Aside from > scalability, the lack of continuations support means: > > 1. JVM TI is not supported when running with --enable-preview (the JVM TI > spec allows for this) > 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs > and so needs JVM TI) > > The VM option "VMContinuations" is added as an experimental option so it can > be used by tests. A number of tests are changed to re-run with > -XX:-VMContinuations. A new jtreg property is added so that tests that need > the underlying VM support to be present can use "@requires vm.continuations" > in the test description. A follow-up change would be to add "@requires > vm.continuations" to the ~70 serviceability/jvmti/vthread that run with > preview features enabled. > Since the package `jdk.internal.access` is > exported[1](#user-content-fn-1-97aea7d7960164849e591e42b91fb5c4) to the > `java.management` module, this can use `MethodHandle`s obtained using the > trusted lookup. That export is for another reason, and probably should be re-examined so that java.base doesn't need to export this package to java.management. In any case, we expect there will be compiler support soon to allow java.management be compiled with code that makes use of preview APIs in java.base. That will at least us reference Thread::isVirtual from code. - PR: https://git.openjdk.java.net/jdk/pull/8939
RFR: 8287496: Alternative virtual thread implementation that maps to OS thread
This patch adds an alternative virtual thread implementation where each virtual thread is backed by an OS thread. It doesn't scale but it can be used by ports that don't have continuations support in the VM. Aside from scalability, the lack of continuations support means: 1. JVM TI is not supported when running with --enable-preview (the JVM TI spec allows for this) 2. jshell --enable-preview can't be used (as jshell uses the debugger APIs and so needs JVM TI) The VM option "VMContinuations" is added as an experimental option so it can be used by tests. A number of tests are changed to re-run with -XX:-VMContinuations. A new jtreg property is added so that tests that need the underlying VM support to be present can use "@requires vm.continuations" in the test description. A follow-up change would be to add "@requires vm.continuations" to the ~70 serviceability/jvmti/vthread that run with preview features enabled. - Commit messages: - Continuation clinit should fail if no continuations support - Fix merge issue with test - Change VMContinuations to be experimental option, ensure JNI GetEnv returns EVERSION - Update - Expend test coverage - Merge - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/8939/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8939=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287496 Stats: 742 lines in 71 files changed: 570 ins; 53 del; 119 mod Patch: https://git.openjdk.java.net/jdk/pull/8939.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8939/head:pull/8939 PR: https://git.openjdk.java.net/jdk/pull/8939
Re: RFR: 8287200: Test java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java timed out after JDK-8287103 [v2]
On Thu, 26 May 2022 22:27:15 GMT, Leonid Mesnik wrote: >> Need to use proper synchronization. >> >> The CyclicBarriers might move the thread to WAITING state but not BLOCKED. >> So it should not confuse existing checks. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > fix Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8874
Re: RFR: 8287200: Test java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java timed out after JDK-8287103
On Wed, 25 May 2022 23:04:50 GMT, Leonid Mesnik wrote:
>> test/jdk/java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java line
>> 100:
>>
>>> 98: while (thread.getState() != Thread.State.BLOCKED) {
>>> 99: Thread.sleep(10);
>>> 100: if (thread.getState() == Thread.State.TERMINATED) {
>>
>> Thread.State.TERMINATED == thread.getState(). Does it make sense?
>
> Not sure. What is the goal?
I assume this test is catch the catch where the thread terminates without
blocking. Using isAlive might be cleaner for people to understand.
-
PR: https://git.openjdk.java.net/jdk/pull/8874
Re: RFR: 8286983: Add mention of "Preview Feature" for new loom related jdb and debug agent options [v2]
On Tue, 24 May 2022 19:36:02 GMT, Chris Plummer wrote: >> I don't think it is correct to say that this command line option is a >> preview feature. Maybe it can be clarified on a second line: >> >> >> -trackvthreadstrack virtual threads as they are created >> Virtual threads are a preview feature of the >> Java platform. > > Does that imply that `-trackvthreads` can be changed or removed? If virtual threads become permanent then the usage message would minimally be updated to drop the sentence that virtual threads are a preview feature. At that point the debugger APIs may have been built out further and it might be that there is no need for the -trackvthreads option but that's too far out to know at this point. If virtual threads were to be removed then the -trackvthreads command line option would be removed too. - PR: https://git.openjdk.java.net/jdk/pull/8780
Re: RFR: 8287200: Test java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java timed out after JDK-8287103
On Tue, 24 May 2022 19:52:57 GMT, Leonid Mesnik wrote: > Need to use proper synchronization. > > The CyclicBarriers might move the thread to WAITING state but not BLOCKED. So > it should not confuse existing checks. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8874
Integrated: 8287008: Improve tests for thread dumps in JSON format
On Thu, 19 May 2022 09:38:06 GMT, Alan Bateman wrote: > This change is mostly test infrastructure and improvements to the testing of > thread dumps in JSON format. It also tweaks the thread dump format so that > the process identifier and thread identifiers are a type string rather than > number. > > The tests for thread dumps added by JEP 425 are changed to use the test > infrastructure library. The tests for JEP 428 will also make use of this test > infrastructure. This pull request has now been integrated. Changeset: 15f15830 Author:Alan Bateman URL: https://git.openjdk.java.net/jdk/commit/15f15830f00895c046c08b55dfeb1618700a2c10 Stats: 431 lines in 4 files changed: 399 ins; 10 del; 22 mod 8287008: Improve tests for thread dumps in JSON format Reviewed-by: cjplummer - PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287008: Improve tests for thread dumps in JSON format [v5]
> This change is mostly test infrastructure and improvements to the testing of > thread dumps in JSON format. It also tweaks the thread dump format so that > the process identifier and thread identifiers are a type string rather than > number. > > The tests for thread dumps added by JEP 425 are changed to use the test > infrastructure library. The tests for JEP 428 will also make use of this test > infrastructure. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8784/files - new: https://git.openjdk.java.net/jdk/pull/8784/files/353fc53d..7e0c1513 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8784=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8784=03-04 Stats: 3 lines in 2 files changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8784.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8784/head:pull/8784 PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8286983: Add mention of "Preview Feature" for new loom related jdb and debug agent options [v2]
On Mon, 23 May 2022 02:52:38 GMT, David Holmes wrote: >> I was really trying to emphasize that the flag is preview, and therefore may >> change or go away. If I change as you suggest, the reader could imply that >> as long as virtual threads support is in place, then the flag will be also >> (and in its current form). > > I agree with @plummercj , this is documenting that the flag is itself a > "preview feature" and thus subject to change. I don't think it is correct to say that this command line option is a preview feature. Maybe it can be clarified on a second line, something like: -trackvthreadstrack virtual threads as they are created Virtual threads are a preview API. - PR: https://git.openjdk.java.net/jdk/pull/8780
Re: RFR: 8287008: Improve tests for thread dumps in JSON format [v4]
On Mon, 23 May 2022 16:44:12 GMT, Chris Plummer wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Use string rater than number for process/thread identifiers > > test/lib/jdk/test/lib/threaddump/ThreadDump.java line 85: > >> 83: * "threads": [...], >> 84: * "threadCount": "1" >> 85: * } > > Why is there no longer a ',' here? I had to collapse several objects and reduce the number of items in the arrays in order to get it down to an example that fits into the class description. To yes, all items except the last should have a trailing comma. - PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287008: Improve tests for thread dumps in JSON format [v4]
On Mon, 23 May 2022 08:30:51 GMT, Alan Bateman wrote: >> This change is mostly test infrastructure and improvements to the testing of >> thread dumps in JSON format. It also tweaks the thread dump format so that >> the process identifier and thread identifiers are a type string rather than >> number. >> >> The tests for thread dumps added by JEP 425 are changed to use the test >> infrastructure library. The tests for JEP 428 will also make use of this >> test infrastructure. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Use string rater than number for process/thread identifiers I've updated the patch to generating the process and thread identifiers as string rather than number (as JSON numbers that are integers don't have the required range). The test infra and DumpThreads.java test are expanded a bit and now test the processId, time and runtimeVersion fields. - PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287008: Improve tests for thread dumps in JSON format [v4]
> This change is mostly test infrastructure and improvements to the testing of > thread dumps in JSON format. It also tweaks the thread dump format so that > the process identifier and thread identifiers are a type string rather than > number. > > The tests for thread dumps added by JEP 425 are changed to use the test > infrastructure library. The tests for JEP 428 will also make use of this test > infrastructure. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Use string rater than number for process/thread identifiers - Changes: - all: https://git.openjdk.java.net/jdk/pull/8784/files - new: https://git.openjdk.java.net/jdk/pull/8784/files/4f9590f3..353fc53d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8784=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8784=02-03 Stats: 58 lines in 4 files changed: 26 ins; 2 del; 30 mod Patch: https://git.openjdk.java.net/jdk/pull/8784.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8784/head:pull/8784 PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287008: Improve tests for thread dumps in JSON format [v3]
On Mon, 23 May 2022 06:09:58 GMT, Chris Plummer wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Remove spurious colon > > test/jdk/com/sun/management/HotSpotDiagnosticMXBean/DumpThreads.java line 115: > >> 113: >> 114: // find the thread container that corresponds to the >> executor >> 115: String name = Objects.toIdentityString(executor); > > I don't understand the need for `toIdentityString()` rather than just > `toString()` The implementation lends on the string returned by Object::toString when not overridden. In this case, either will do so I can change it to toString. > test/lib/jdk/test/lib/threaddump/ThreadDump.java line 179: > >> 177: return this; >> 178: if (name().startsWith(name + "/")) >> 179: return this; > > It's not clear to me why this is here. What does it mean if the name of the > container starts with `/`? With the exception of the root container, the names have the form "name1/name2" where name1 is the "external name" and name2 identifies the internal thread grouping. The external name is the object identity for thread pools and thread-per-executors created by user code. In time we may expose ways to provide a name. > test/lib/jdk/test/lib/threaddump/ThreadDump.java line 299: > >> 297: >> 298: // add to map if not already encountered >> 299: var container = map.computeIfAbsent(name, k -> new >> ThreadContainer(name)); > > Why might the container already be in `map`? They aren't topologically sorted so it's possible to encounter a thread container before its parent. - PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287103: java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java fails with Xcomp [v2]
On Sat, 21 May 2022 16:34:32 GMT, Leonid Mesnik wrote: >> Sync improved in test > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > fix Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8821
Re: RFR: 8287103: java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java fails with Xcomp
On Fri, 20 May 2022 22:27:29 GMT, Leonid Mesnik wrote: > Sync improved in test test/jdk/java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java line 43: > 41: private static final Object LOCK2 = new Object(); > 42: > 43: private static volatile boolean lock2Obtained = false; The term used in the APIs is "lock held" so might be better to rename this to lock2Held. Can you drop the explicit init to false and also put a blank line after the declaration as it looks strange to have it run into the comment on the main method. - PR: https://git.openjdk.java.net/jdk/pull/8821
Re: RFR: 8287103: java/lang/management/ThreadMXBean/VirtualThreadDeadlocks.java fails with Xcomp
On Fri, 20 May 2022 22:27:29 GMT, Leonid Mesnik wrote: > Sync improved in test Can you add `@bug 8284161 8287103` to the test description? - PR: https://git.openjdk.java.net/jdk/pull/8821
Re: RFR: 8287008: Improve tests for thread dumps in JSON format [v3]
> This is a test-only change to add some test infrastructure and improve the > testing of thread dumps in JSON format. The new tests added by JEP 425 for > this thread dump format search the JSON text for strings but don't parse it > completely. These tests can be improved with a test class that parses the > thread dump. The tests for JEP 428 like make use of the test infrastructure > too. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Remove spurious colon - Changes: - all: https://git.openjdk.java.net/jdk/pull/8784/files - new: https://git.openjdk.java.net/jdk/pull/8784/files/15c2fd7b..4f9590f3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8784=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8784=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8784.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8784/head:pull/8784 PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287008: Improve tests for thread dumps in JSON format
On Fri, 20 May 2022 04:22:21 GMT, Chris Plummer wrote: > I think a short example dump would actually be easier to read alongside the > code. That's a good idea. I've updated the class description to include an example and also a code example of how it can be used in tests. The thread dump example collapses several objects to avoid it getting too unwieldily. I've started [PR8807](https://github.com/openjdk/jdk/pull/8807) to document the format. - PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287008: Improve tests for thread dumps in JSON format [v2]
> This is a test-only change to add some test infrastructure and improve the > testing of thread dumps in JSON format. The new tests added by JEP 425 for > this thread dump format search the JSON text for strings but don't parse it > completely. These tests can be improved with a test class that parses the > thread dump. The tests for JEP 428 like make use of the test infrastructure > too. Alan Bateman has updated the pull request incrementally with one additional commit since the last revision: Add example thread dump and usage to class description - Changes: - all: https://git.openjdk.java.net/jdk/pull/8784/files - new: https://git.openjdk.java.net/jdk/pull/8784/files/c0a12746..15c2fd7b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8784=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8784=00-01 Stats: 69 lines in 1 file changed: 68 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8784.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8784/head:pull/8784 PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287008: Improve tests for thread dumps in JSON format
On Thu, 19 May 2022 09:38:06 GMT, Alan Bateman wrote:
> This is a test-only change to add some test infrastructure and improve the
> testing of thread dumps in JSON format. The new tests added by JEP 425 for
> this thread dump format search the JSON text for strings but don't parse it
> completely. These tests can be improved with a test class that parses the
> thread dump. The tests for JEP 428 like make use of the test infrastructure
> too.
If we use JSON schema then it will something like this, does that help?
{
"properties": {
"threadDump": {
"type": "object",
"properties": {
"processId": {
"type": "integer"
},
"time": {
"type": "string"
},
"runtimeVersion": {
"type": "string"
},
"threadContainers": {
"type": "array",
"items": [
{
"type": "object",
"properties": {
"tid": {
"type": "integer"
},
"name": {
"type": "string"
},
"stack": {
"type": "array",
"items": [
{
"type": "string"
}
]
}
},
"required": [
"tid",
"name",
"stack"
]
}
]
}
},
"required": [
"processId",
"time",
"runtimeVersion",
"threadContainers"
]
}
},
"required": [
"threadDump"
]
}
-
PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8287008: Improve tests for thread dumps in JSON format
On Thu, 19 May 2022 15:28:01 GMT, Chris Plummer wrote: > It would be useful if ThreadDump.java contained a comment with a description > of the JSON file syntax. It could either be in the form of a simple dump or > as a simple syntactic description. Still TBD on where the JSON "schema" will be specified, it might HotSpotDiagnosMXBean.dumpThreads so it will needed by tools that parse the JSON text. So when we do that then I think we can reference it from the test class. - PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8286983: Add mention of "Preview Feature" for new loom related jdb and debug agent options
On Thu, 19 May 2022 00:10:15 GMT, Chris Plummer wrote: > As part of the loom integration, jdb added -trackvthreads and the debug agent > added "enumeratevthreads". The help text for these options should call out > that they are Preview Features. src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c line 880: > 878: "onuncaught=y|n debug on any uncaught?n\n" > 879: "timeout= for listen/attach in milliseconds n\n" > 880: "enumeratevthreads=y|n(Preview Feature) thread lists > include all vthreads\n" I think the description needs to say "virtual threads" rather than "vthreads". Also I think "(Preview" needs to go after "virtual threads" to make it clearer that it's virtual threads that are preview,. - PR: https://git.openjdk.java.net/jdk/pull/8780
RFR: 8287008: Improve tests for thread dumps in JSON format
This is a test-only change to add some test infrastructure and improve the testing of thread dumps in JSON format. The new tests added by JEP 425 for this thread dump format search the JSON text for strings but don't parse it completely. These tests can be improved with a test class that parses the thread dump. The tests for JEP 428 like make use of the test infrastructure too. - Commit messages: - Initial commit Changes: https://git.openjdk.java.net/jdk/pull/8784/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8784=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287008 Stats: 332 lines in 3 files changed: 305 ins; 9 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/8784.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8784/head:pull/8784 PR: https://git.openjdk.java.net/jdk/pull/8784
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() [v2]
On Wed, 11 May 2022 02:43:21 GMT, Ioi Lam wrote: >> The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never >> supported the value `"rw"` since the source code was imported to the openjdk >> repo more than 15 years ago. In fact HotSpot throws >> `IllegalArgumentException` when such a mode is specified. >> >> It's unlikely such a mode will be required for future enhancements. Support >> for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` >> is the only supported mode. >> >> I also cleaned up related code in the JDK and HotSpot. >> >> Testing: >> - Passed tiers 1 and 2 >> - Tiers 3, 4, 5 are in progress > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > review comments Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach()
On Tue, 10 May 2022 04:00:29 GMT, Ioi Lam wrote: > The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never > supported the value `"rw"` since the source code was imported to the openjdk > repo more than 15 years ago. In fact HotSpot throws > `IllegalArgumentException` when such a mode is specified. > > It's unlikely such a mode will be required for future enhancements. Support > for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` > is the only supported mode. > > I also cleaned up related code in the JDK and HotSpot. > > Testing: > - Passed tiers 1 and 2 > - Tiers 3, 4, 5 are in progress I skimmed through the changes and I think they look okay. In the distant past there were tools outside of the JDK that used the jvmstat API directly. It's possible that VisualVM still does but it would only compile/run if --add-exports is used to export the sun.jvmstat.* packages. So it might be that dropping the parameter from a method in RemoteHost is noticed and I think that is okay because this package is not exported and is not meant to be used by code outside of the JDK. - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach()
On Tue, 10 May 2022 04:00:29 GMT, Ioi Lam wrote:
> The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never
> supported the value `"rw"` since the source code was imported to the openjdk
> repo more than 15 years ago. In fact HotSpot throws
> `IllegalArgumentException` when such a mode is specified.
>
> It's unlikely such a mode will be required for future enhancements. Support
> for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"`
> is the only supported mode.
>
> I also cleaned up related code in the JDK and HotSpot.
>
> Testing:
> - Passed tiers 1 and 2
> - Tiers 3, 4, 5 are in progress
src/jdk.internal.jvmstat/share/classes/sun/jvmstat/perfdata/monitor/protocol/file/PerfDataBuffer.java
line 60:
> 58: FileChannel fc = new RandomAccessFile(f, "r").getChannel();
> 59: ByteBuffer bb = fc.map(FileChannel.MapMode.READ_ONLY, 0L,
> (int)fc.size());
> 60: fc.close(); // doesn't need to remain open
I think you can change this to:
try (FileChannel fc = FileChannel.open(f.toPath())) {
ByteBuffer bb = ...
createPerfDataBuffer(bb, 0);
}
-
PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8278123: serviceability/dcmd/vm/ClassLoaderStatsTest.java failing with java.lang.AssertionError: Should have a hidden class [v2]
On Thu, 28 Apr 2022 21:58:46 GMT, Leonid Mesnik wrote:
>> The test failed if GC happens somewhere between
>> Class c = Class.forName("TestClass", true, dummyloader);
>> and
>> OutputAnalyzer output = executor.execute("VM.classloader_stats");
>>
>> The fix is to make hc static as Chris proposed.
>>
>> To verfiy fix I add System.gc() before
>> executor.execute("VM.classloader_stats");
>
> Leonid Mesnik has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains three additional
> commits since the last revision:
>
> - comment fixed
> - Merge branch 'master' of https://github.com/openjdk/jdk into 8278123
> - fix
I assume you'll remove this test from the-Xcomp exclude list
(test/hotspot/jtreg/ProblemList-Xcomp.txt) as part of this change,
-
PR: https://git.openjdk.java.net/jdk/pull/8438
Integrated: 8284161: Implementation of Virtual Threads (Preview)
On Fri, 8 Apr 2022 13:43:39 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. This pull request has now been integrated. Changeset: 9583e365 Author:Alan Bateman URL: https://git.openjdk.java.net/jdk/commit/9583e3657e43cc1c6f2101a64534564db2a9bd84 Stats: 99468 lines in 1133 files changed: 91198 ins; 3598 del; 4672 mod 8284161: Implementation of Virtual Threads (Preview) Co-authored-by: Ron Pressler Co-authored-by: Alan Bateman Co-authored-by: Erik Österlund Co-authored-by: Andrew Haley Co-authored-by: Rickard Bäckman Co-authored-by: Markus Grönlund Co-authored-by: Leonid Mesnik Co-authored-by: Serguei Spitsyn Co-authored-by: Chris Plummer Co-authored-by: Coleen Phillimore Co-authored-by: Robbin Ehn Co-authored-by: Stefan Karlsson Co-authored-by: Thomas Schatzl Co-authored-by: Sergey Kuksenko Reviewed-by: lancea, eosterlund, rehn, sspitsyn, stefank, tschatzl, dfuchs, lmesnik, dcubed, kevinw, amenkov, dlong, mchung, psandoz, bpb, coleenp, smarks, egahlin, mseledtsov, coffeys, darcy - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v14]
> 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 23 commits: - Refresh 4e99b5185eef9398c4cc4e90544de4a3153d61a9 - Merge with 5212535a276a92d96ca20bdcfccfbce956febdb1 - Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3 - Merge with 9425ab2b43b649bd591706bfc820e9c8795a6fdf - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - ... and 13 more: https://git.openjdk.java.net/jdk/compare/5212535a...df43e6fc - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=13 Stats: 99468 lines in 1133 files changed: 91198 ins; 3598 del; 4672 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v13]
On Fri, 6 May 2022 06:48:46 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 21 commits: > > - Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3 > - Merge with 9425ab2b43b649bd591706bfc820e9c8795a6fdf > - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f > - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 > - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 > - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 > - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a > - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 > - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec > - Merge with jdk-19+20 > - ... and 11 more: > https://git.openjdk.java.net/jdk/compare/9425ab2b...6d968671 > It is understandable to ship the preview feature not implemented on all > platforms. It is even understandable to ship the final product feature that > breaks some platforms in an clearly understandable way, prompting platform > maintainers to implement the agreed-upon, final Java feature. What I am > seeing, though, is that just running `java -version` (!) after integrating a > _preview feature_ is broken! Preview features need to be implemented by a port before it can be released. For JDK 19 this means that the maintainers of ports will have work to do for both JEP 424 and JEP 425 (ifdef is not an option). I think the issue that are concerned about is the interim period between the JEP 425 integration and the port/implementation of this feature to other architectures. I think the answer is that it will vary. It may be that some port maintainers decide to do something very short term so they can run without --enable-preview and buy time before they do the port/implementation for JDK 19. Good progress has been reported on at least ppc64le port and maybe that port be ready before others. So yes, some ports may crash until they receive attention, others (like zero) should be able to run tests that don't use --enable-preview. I've no doubt there will be a flurry of changes post integration. The motivation for Continuations::enabled() was to reduce risk and disable a lot of the new code by default. The motivation wasn't porting but it may be helpful during the interim period. That is probably a topic for loom-dev rather here. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v13]
> 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 21 commits: - Refresh 6ace49bf42e5504c69fa11c564e8e865c0a95fb3 - Merge with 9425ab2b43b649bd591706bfc820e9c8795a6fdf - Refresh 12aa09ce7efd48425cc080d0b8761aca0f3e215f - Merge with 1bb4de2e2868a539846ec48dd43fd623c2ba69a5 - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec - Merge with jdk-19+20 - ... and 11 more: https://git.openjdk.java.net/jdk/compare/9425ab2b...6d968671 - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=12 Stats: 99456 lines in 1130 files changed: 91188 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
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) [v11]
On Wed, 4 May 2022 16:02:36 GMT, Aleksey Shipilev wrote: > So, does this PR pass current GHA checks? I see they are not enabled for this > PR. It would be unfortunate for this large integration to break builds/tests > for smaller PRs that would follow it. I've enabled it on my fork and we'll see if it kicks in. - 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v11]
> 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 17 commits: - Refresh d77f7a49af75bcc5b20686805ff82a93a20dde05 - Merge with 4b2c82200fdc01de868cf414e40a4d891e753f89 - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec - Merge with jdk-19+20 - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 - Refresh cf561073f48fad58e931a5285b92629aa83c9bd1 - Merge with jdk-19+19 - ... and 7 more: https://git.openjdk.java.net/jdk/compare/4b2c8220...f06aff75 - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=10 Stats: 102779 lines in 1140 files changed: 92909 ins; 4219 del; 5651 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v10]
> 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 15 commits: - Refresh 6091080db743ece5f1b2111fcc35a5f2179a403a - Merge with cfcba1fccc8e3e6a68e1cb1826b70e076d5d83c4 - Refresh ee9fa8ed05ec22de7a13383052d68aa8aa7832ec - Merge with jdk-19+20 - Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e - Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 - Refresh cf561073f48fad58e931a5285b92629aa83c9bd1 - Merge with jdk-19+19 - Refresh - Refresh - ... and 5 more: https://git.openjdk.java.net/jdk/compare/cfcba1fc...f41b3131 - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=09 Stats: 102769 lines in 1140 files changed: 92907 ins; 4216 del; 5646 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
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 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: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v4]
On Thu, 28 Apr 2022 16:58:40 GMT, Joe Darcy wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to more review feedback. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8410
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: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]
On Thu, 28 Apr 2022 01:34:19 GMT, Joe Darcy wrote: >> To enable more complete doclint checking (courtesy @jonathan-gibbons), >> please review this PR to add type-level @param tags where they are missing. >> >> To the maintainers of java.util.concurrent, those changes could be separated >> out in another bug if that would ease maintenance of that code. >> >> Making these library fixes is a blocker for correcting and expanding the >> doclint checks (JDK-8285496). >> >> I'll update copyright years before pushing. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to more review feedback. src/java.base/share/classes/java/nio/file/SecureDirectoryStream.java line 55: > 53: * against the original path of the directory (irrespective of if > the > 54: * directory is moved since it was opened). > 55: * @param the type of path It may not be a path. The type parameter is specified in the super interfaces, can you copy that down instead? src/java.base/share/classes/java/nio/file/WatchEvent.java line 51: > 49: /** > 50: * An event kind, for the purposes of identification. > 51: * @param the type of the context value This is okay but the it differs slightly to the type parameters specified further up. I think the issue here is that it was just wasn't copied down to WatchEvent.Kind. - PR: https://git.openjdk.java.net/jdk/pull/8410
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: 8278123: serviceability/dcmd/vm/ClassLoaderStatsTest.java failing with java.lang.AssertionError: Should have a hidden class
On Thu, 28 Apr 2022 04:46:35 GMT, Chris Plummer wrote: >> This line added by 8238358: Implementation of JEP 371: Hidden Classes which >> has many co-authors. Hope someone could provide an explanation during this >> review. >> >> It might be possible that the goal was to verify that VM.classloader_stats >> provide might provide info for non-reachable clasees. However it makes test >> to fragile, since can't block class unloading now. > > Yes, that seems to be the case, and you are right that it is not something > that is safe to assume. Lookup.defineHiddenClass allows class options to be specified, one of which is "STRONG" to mean that the hidden class can't unloaded if its defining loader is reachable. A static reference or a reachability fence should work for this test. - PR: https://git.openjdk.java.net/jdk/pull/8438
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 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]
> 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=8166=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8166=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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v7]
On Wed, 27 Apr 2022 11:34:51 GMT, Daniel Fuchs wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69
>
> src/jdk.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java
> line 149:
>
>> 147: * access to the file or {@link
>> java.lang.management.ManagementPermission
>> 148: * ManagementPermission("control")} is denied
>> 149: * @since 19
>
> Maybe there ought to be an `@throws UnsupportedOperationException` here since
> that is what the default implementation of the method is supposed to do.
Well spotted. The implSpec further up documents it but it should be in the
throws list too.
> test/jdk/java/net/vthread/HttpALot.java line 76:
>
>> 74: var address = server.getAddress();
>> 75: URL url = new URL("http://; + address.getHostName() + ":" +
>> address.getPort() + "/hello");
>> 76:
>
> Nit: Ideally we should use the URIBuilder from the test library here, and the
> IP literal address to improve stability of the test on machines that may have
> strange /etc/hosts configuration. This can be taken care of after this PR has
> been integrated.
A few of these tests started out as standalone classes that could be run
without test infrastructure. This one, and InterruptHttp, can be easily changed
to use the URIBuilder infra library, so we can do that.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v7]
On Tue, 26 Apr 2022 18:58:23 GMT, Daniel Fuchs wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69
>
> src/java.base/share/classes/sun/nio/ch/DatagramChannelImpl.java line 770:
>
>> 768: synchronized (p) {
>> 769: DatagramPackets.setLength(p, n);
>> 770: p.setSocketAddress(sender);
>
> Hmmm... For integrity it might be better to call the public
> `DatagramPacket::setData(byte[] buf, int offset, int length)` method here
> now. The additional advantage is that the private access to
> `DatagramPackets.setLength(p, n);` will no longer be needed.
DatagramPackets.setLength is not really new, it's just moved. There's a flaw in
DatagramPacket that forces its usage - the details are in JDK-8232817.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v7]
> 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 8d8f0a2fd646e57fe6b4e8ab669f836dc46dda69 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8166/files - new: https://git.openjdk.java.net/jdk/pull/8166/files/05781877..0864cb09 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8166=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8166=05-06 Stats: 582 lines in 34 files changed: 285 ins; 128 del; 169 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v6]
> 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 with a new target base due to a merge or a rebase. The pull request now contains ten commits: - Refresh cf561073f48fad58e931a5285b92629aa83c9bd1 - Merge with jdk-19+19 - Refresh - Refresh - Refresh - Refresh - Merge with jdk-19+18 - Refresh - Initial push - Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=05 Stats: 104157 lines in 1144 files changed: 94155 ins; 4291 del; 5711 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Fri, 15 Apr 2022 21:24:58 GMT, Paul Sandoz wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh > > src/java.base/share/classes/jdk/internal/vm/Continuation.java line 115: > >> 113: } >> 114: >> 115: private Runnable target; > > Can be final? (Does not appear to be updated by the VM, or via unsafe by some > other class) Yes, it can. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v5]
On Fri, 22 Apr 2022 02:26:50 GMT, ExE Boss wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh
>
> src/java.base/share/classes/java/lang/ThreadLocal.java line 179:
>
>> 177: private T get(Thread t) {
>> 178: ThreadLocalMap map = getMap(t);
>> 179: if (map != null && map != ThreadLocalMap.NOT_SUPPORTED) {
>
> Due to the way `setInitialValue` is implemented, `getMap(t)` will currently
> be called twice when `ThreadLocal`s are disabled.
>
>
>
> This method should probably be changed so that when `map ==
> ThreadLocalMap.NOT_SUPPORTED`, it simply does:
>
> return initialValue();
>
>
>
>
> Suggestion:
>
> if (map != null) {
> if (map == ThreadLocalMap.NOT_SUPPORTED) {
> return initialValue();
> }
It's benign but what you suggest may be clearer - thanks!
> src/java.base/share/classes/java/lang/ThreadLocal.java line 423:
>
>> 421: * Construct a new map without a table.
>> 422: */
>> 423: ThreadLocalMap() {
>
> It might be possible for this to be `private`:
> Suggestion:
>
> private ThreadLocalMap() {
Yes, this can be private.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Fri, 15 Apr 2022 21:31:09 GMT, Paul Sandoz wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh
>
> src/java.base/share/classes/jdk/internal/vm/Continuation.java line 264:
>
>> 262: } finally {
>> 263: fence();
>> 264: StackChunk c = tail;
>
> `c` is not used
Ron has done some cleanup so this is removed.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Sat, 16 Apr 2022 14:59:55 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/jdk/internal/vm/ThreadContainers.java line 184:
>>
>>> 182: * with the Thread API.
>>> 183: */
>>> 184: private static class RootContainer extends ThreadContainer {
>>
>> This implementation could be clearer if split out into two, and the value
>> assigned to `INSTANCE` is selected based on the system property. Something
>> to consider later perhaps.
>
> We've been undecided on whether to just track all threads. If we do that then
> there would be only one implementation. But yes, if we continue with two then
> they should be split out.
Done.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v4]
On Tue, 19 Apr 2022 01:11:56 GMT, Mandy Chung wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh > > src/java.base/share/classes/java/lang/System.java line 2173: > >> 2171: >> 2172: // start Finalizer and Reference Handler threads >> 2173: SharedSecrets.getJavaLangRefAccess().startThreads(); > > I think it would avoid any confusion if `VM.initLevel(1)` is the last step in > `initPhase1`, i.e. call after this line. Previously, the finalizer starts > very early and it has to wait until initLevel is set to level 1 which > guarantees that `JavaLangAccess` is available. With this change, > `JavaLangAccess` is already set. Yes, that would be best, just wasn't possible before now. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v5]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8166/files - new: https://git.openjdk.java.net/jdk/pull/8166/files/ff1ef712..5e202eca Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8166=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8166=03-04 Stats: 1827 lines in 289 files changed: 682 ins; 377 del; 768 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v4]
On Tue, 19 Apr 2022 00:09:04 GMT, Mandy Chung wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh
>
> src/java.base/share/classes/java/lang/ref/ReferenceQueue.java line 61:
>
>> 59: private final Condition notEmpty;
>> 60:
>> 61: void signal() { notEmpty.signalAll(); }
>
> `signal()`, `await()` and `await(long)` methods are only used by
> `ReferenceQueue`. Good to make them private.
They are overridden so can't be private.
> src/java.base/share/classes/java/lang/ref/ReferenceQueue.java line 206:
>
>> 204: throws IllegalArgumentException, InterruptedException {
>> 205: if (timeout < 0) throw new IllegalArgumentException("Negative
>> timeout value");
>> 206: if (timeout == 0) return remove();
>
> Nit: use the same formatting as in `NativeReferenceQueue::remove` and in this
> file.
>
>
> if (timeout < 0)
> throw new IllegalArgumentException("Negative timeout value");
> if (timeout == 0)
> return remove();
okay
> src/java.management/share/classes/java/lang/management/ThreadMXBean.java line
> 655:
>
>> 653: * synchronization control. It might be an expensive operation.
>> 654: * Its behavior with cycles involving virtual threads is not defined
>> 655: * in this release.
>
> What does the current implementation return if the virtual threads are
> involved in a deadlock?The class spec says "ThreadMXBean does not support
> monitoring or management of virtual threads" while this javadoc says it's
> undefined.I wonder if it's helpful to include `@implNote` if appropriate.
If there is cycle involving a virtual thread then the thread id of its carrier
will be included in array. I think we can do better and include the thread id
of the mounted thread.
> src/jdk.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java
> line 138:
>
>> 136: *
>> 137: * @param outputFile the path to the file to create
>> 138: * @param format the format to use (TEXT_PLAIN or JSON)
>
> It's useful to link to `ThreadDumpFormat#TEXT_PLAN` and
> `ThreadDumpFormat#JSON`
I think it might be better if the description is changed to "the format to
use". There is already a link to the ThreadDumpFormat enum in method signature.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Fri, 15 Apr 2022 21:15:45 GMT, Paul Sandoz wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh
>
> src/java.base/share/classes/jdk/internal/vm/Continuation.java line 94:
>
>> 92: default:
>> 93: throw new AssertionError("Unknown pinned reason: " +
>> reason);
>> 94: }
>
> Suggestion:
>
> return switch (reason) {
> case 2 -> Pinned.CRITICAL_SECTION;
> case 3 -> Pinned.NATIVE;
> case 4 -> Pinned.MONITOR;
> default -> throw new AssertionError("Unknown pinned reason: " +
> reason);
> };
That would be clearer (I think this predates switch expressions).
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v4]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8166/files - new: https://git.openjdk.java.net/jdk/pull/8166/files/f53f0d4e..ff1ef712 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8166=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8166=02-03 Stats: 1517 lines in 74 files changed: 1034 ins; 311 del; 172 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
On Sun, 17 Apr 2022 12:39:24 GMT, Jaikiran Pai wrote:
> I had a look at the referenced
> [JDK-4006245](https://bugs.openjdk.java.net/browse/JDK-4006245). The
> previous/existing reference cleaning logic seems to have been added mainly to
> `null` the reference to the user/application class instance represented by
> the `Runnable` task. Of course, other references too were being cleared. The
> description in the linked JBS seems to suggest that the `Thread` instance
> wasn't being GCed sooner. That was more than 2 decades back. I don't have
> enough knowledge of this area to know if this is still an issue in recent GC
> implementations.
The comment on that method dates from early JDK releases with the Classic VM
and its conservative GC. It also predates the java.lang.ref API. The question
that I think you are asking is if you create a Thread with a runnable task, let
it terminate but keep the reference to the Thread forever then what does it
keep alive. We can think about more but so far we haven't seen any issues or
tests fail with the reference to the runnable task.
> src/java.base/share/classes/java/lang/Thread.java line 2414:
>
>> 2412: sm.checkPermission(new
>> RuntimePermission("setContextClassLoader"));
>> 2413: }
>> 2414: if (!isSupportedClassLoader(contextClassLoader)) {
>
> For a few moments, I thought this was a typo and should instead have been:
>
> if (!isSupportedClassLoader(cl)) {
>
> but then thinking a bit more, I realized what's being done here. Essentially,
> this checks whether the _current_ context classloader is set to "not allowed
> to change/set context classloader" (represented by
> `Constants.NOT_SUPPORTED_CLASSLOADER`).
>
> Do you think this line deserves some short code comment that it intentionally
> passes the current contextClassLoader (and not the one passed to the method)
> to `isSupportedClassLoader`?
I think the comment on isSupportedClassLoader is readable so I think it's okay.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
On Sun, 17 Apr 2022 03:49:19 GMT, Jaikiran Pai wrote:
>> Alan Bateman has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - Refresh
>> - Refresh
>> - Merge with jdk-19+18
>> - Refresh
>> - Initial push
>
> src/java.base/share/classes/java/io/PrintStream.java line 1420:
>
>> 1418: } else {
>> 1419: synchronized (this) {
>> 1420: implFormat(format, args);
>
> I think there's a typo here. I think it should have been:
>
>
> implFormat(l, format, args);
Well spotted, the locale should be provided.
> src/java.base/share/classes/java/lang/Thread.java line 602:
>
>> 600: } else {
>> 601: // getContextClassLoader not trusted
>> 602: ClassLoader cl = parent.contextClassLoader;
>
> I might be misreading the comment on that line, but reading this if/else
> block a few times, I'm unsure what the expectations here are.
>
> It's my understanding that a call to `getContextClassLoader()` can't be
> trusted if that method has been overridden by the passed `Thread` type. In
> such cases, we don't call that method and instead just use the field value of
> `contextClassLoader` (which is a private field on `Thread`). Is that
> understanding correct? If yes, then the condition in the `if` block a few
> lines above, looks odd. It seems to be calling the `getContextClassLoader()`
> if that method is overridden by the passed `Thread` type, i.e. the untrusted
> case. Should it instead be:
>
> if (sm == null || !isCCLOverridden(parent.getClass())) {
> return parent.getContextClassLoader();
> }
>
> (notice the negation)
This code hasn't changed, it's just moved to a helper method. When running with
a security manager and the CCL methods aren't overridden, then it avoids the
permission check. However, I can see how the comment is mis-reading so we can
improve that.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
On Sat, 16 Apr 2022 09:25:20 GMT, Jaikiran Pai wrote:
>> Alan Bateman has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - Refresh
>> - Refresh
>> - Merge with jdk-19+18
>> - Refresh
>> - Initial push
>
> src/java.base/share/classes/java/io/BufferedOutputStream.java line 136:
>
>> 134: * are added. A no-op if the buffer is not resizable.
>> 135: */
>> 136: private void growIfNeeded(int len) {
>
> Hello Alan, like some methods in other classes (BufferedInputStream#fill()
> for example), should we mention here that this method assumes that it's being
> called while a lock is held?
Okay, we can add a comment to these methods (asserts are possible but tend not
to be used in this area).
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Fri, 15 Apr 2022 22:01:10 GMT, Paul Sandoz wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh
>
> src/java.base/share/classes/jdk/internal/vm/ThreadContainers.java line 184:
>
>> 182: * with the Thread API.
>> 183: */
>> 184: private static class RootContainer extends ThreadContainer {
>
> This implementation could be clearer if split out into two, and the value
> assigned to `INSTANCE` is selected based on the system property. Something
> to consider later perhaps.
We've been undecided on whether to just track all threads. If we do that then
there would be only one implementation. But yes, if we continue with two then
they should be split out.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Sat, 16 Apr 2022 12:31:41 GMT, ExE Boss wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh
>
> src/java.base/share/classes/jdk/internal/misc/Blocker.java line 111:
>
>> 109: methodType = MethodType.methodType(void.class,
>> long.class);
>> 110: endCompensatedBlock = l.findVirtual(ForkJoinPool.class,
>> "endCompensatedBlock", methodType);
>> 111:
>
> This can use `SharedSecrets.getJavaLangInvokeAccess()` in order to avoid
> using `privateLookupIn(…)` and `AccessController.doPrivileged(…)`.
>
> Suggestion:
>
> JavaLangInvokeAccess JLIA =
> SharedSecrets.getJavaLangInvokeAccess();
> MethodType methodType = MethodType.methodType(long.class);
> beginCompensatedBlock = JLIA.findVirtual(ForkJoinPool.class,
> "beginCompensatedBlock", methodType);
> if (beginCompensatedBlock == null) {
> throw new NoSuchMethodException(ForkJoinPool.class +
> ".beginCompensatedBlock" + methodType);
> }
> methodType = MethodType.methodType(void.class, long.class);
> endCompensatedBlock = JLIA.findVirtual(ForkJoinPool.class,
> "endCompensatedBlock", methodType);
> if (endCompensatedBlock == null) {
> throw new NoSuchMethodException(ForkJoinPool.class +
> ".endCompensatedBlock" + methodType);
> }
I think we may just eliminate the reflection use and use a shared secret
instead.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
On Sat, 16 Apr 2022 11:05:47 GMT, Jaikiran Pai wrote:
>> Alan Bateman has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - Refresh
>> - Refresh
>> - Merge with jdk-19+18
>> - Refresh
>> - Initial push
>
> src/java.base/share/classes/java/io/FilterInputStream.java line 233:
>
>> 231: */
>> 232: @Override
>> 233: public void reset() throws IOException {
>
> I suspect this change to remove synchronization from `mark()` and `reset()`
> is intentional. The `FilterInputStream` class doesn't explain the thread
> safety semantics, nor can I find commit history which would hint on why these
> (and only these) methods were syncrhonized. But it looks like this
> synchronization was there to serialize calls to `mark()` and `reset()` on the
> same `FilterInputStream` from multiple threads. With this synchronization now
> removed, subclasses or even direct usages of `FilterInputStream` would now be
> impacted. Potentially outside of the JDK. Would that be a concern?
Yes, this is intentional but it would be great to split out this change and get
them into the main line in advance. There is a JEP size task required to
re-visit all of the locking in java.io. In the case of the input/output
streams, most of it is not documented and there are several inconsistencies. In
the case of FilterInputStream, it doesn't use synchronization for the other ops.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
On Sat, 16 Apr 2022 10:25:56 GMT, Jaikiran Pai wrote:
>> Alan Bateman has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>> commits since the last revision:
>>
>> - Refresh
>> - Refresh
>> - Merge with jdk-19+18
>> - Refresh
>> - Initial push
>
> src/java.base/share/classes/jdk/internal/misc/Blocker.java line 76:
>
>> 74: && currentCarrierThread() instanceof CarrierThread ct &&
>> !ct.inBlocking()) {
>> 75: ct.beginBlocking();
>> 76: long comp =
>> ForkJoinPools.beginCompensatedBlock(ct.getPool());
>
> It appears that `ForkJoinPools.beginCompensatedBlock(...)` can throw an
> exception. Would such an exception require the CarrierThread's blocking state
> (which we set on the previous line) to be reset by a call to
> `ct.endBlocking()`? Or is it futile to reset that state if any exception gets
> thrown from the call to `ForkJoinPools.beginCompensatedBlock(...)`?
REE isn't possible here but maybe you mean a resource issues such as stack
overflow or OOME. If that happens then the flag wouldn't be reset. It wouldn't
a corrects issue but we may be able to make this a bit more robust for these
cases.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
On Sat, 16 Apr 2022 10:29:36 GMT, Jaikiran Pai wrote: >> Alan Bateman has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains five additional >> commits since the last revision: >> >> - Refresh >> - Refresh >> - Merge with jdk-19+18 >> - Refresh >> - Initial push > > src/java.base/share/classes/jdk/internal/misc/Blocker.java line 94: > >> 92: >> 93: /** >> 94: * Marks the end an operation that may have blocked. > > I think this should have been "Marks the end of an ..." Yes, there's a typo there - thanks. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Fri, 15 Apr 2022 20:39:51 GMT, Paul Sandoz wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh
>
> src/java.base/share/classes/jdk/internal/misc/Blocker.java line 126:
>
>> 124: static void endCompensatedBlock(ForkJoinPool pool, long post) {
>> 125: try {
>> 126: endCompensatedBlock.invoke(pool, post);
>
> Suggestion:
>
> endCompensatedBlock.invokeExact(pool, post);
Yes, that would be better.
> src/java.base/share/classes/jdk/internal/misc/InternalLock.java line 46:
>
>> 44: } else {
>> 45: CAN_USE_INTERNAL_LOCK = true;
>> 46: }
>
> Suggestion:
>
> CAN_USE_INTERNAL_LOCK = Boolean.getBoolean("jdk.io.useMonitors");
This doesn't work for the empty string case as it would map to true rather
than false. So I think we leave this as is.
> src/java.base/share/classes/jdk/internal/misc/ThreadFlock.java line 262:
>
>> 260: * @return the thread, started
>> 261: * @throws IllegalStateException if this flock is shutdown or closed
>> 262: * @throws jdk.incubator.concurrent.StructureViolationException if
>> the current
>
> Suggestion:
>
> * @throws WrongThreadException if the current thread is not the owner or
> a thread
> * contained in the flock
> * @throws jdk.incubator.concurrent.StructureViolationException if the
> current
Thanks, I think this slipped in when moving the SL API to the incubator module.
It should be IllegalThreadStateException if the thread has already started, and
WrongThreadException if not the owner (or flock).
> src/java.base/share/classes/jdk/internal/misc/ThreadTracker.java line 59:
>
>> 57: && this.thread == other.thread;
>> 58: }
>> 59: }
>
> Suggestion:
>
> private record ThreadRef(Thread thread) {
> @Override
> public int hashCode() {
> return Long.hashCode(thread.threadId());
> }
>
> @Override
> public boolean equals(Object obj) {
> return (obj instanceof ThreadRef other)
> && this.thread == other.thread;
> }
> }
Yes, this can be a record.
> src/java.base/share/classes/jdk/internal/vm/annotation/ChangesCurrentThread.java
> line 35:
>
>> 33: * disables inlining for the method to which it is applied unless the
>> 34: * method being unlined into is also annotated ChangesCurrentThread.
>> 35:
>
> Suggestion:
>
> * method being inlined into is also annotated ChangesCurrentThread.
> *
Thanks.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
On Fri, 15 Apr 2022 12:31:24 GMT, Andrey Turbanov wrote: > method seems unused now It's used by the SL API, which will follow later. I realize this may sound complicated but it avoids more complicated splits. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Thu, 14 Apr 2022 21:39:17 GMT, Paul Sandoz wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Refresh
>
> src/java.base/share/classes/java/lang/Thread.java line 862:
>
>> 860: * @param start the starting value of the counter
>> 861: * @return this builder
>> 862: * @throws IllegalArgumentException if count is negative
>
> Suggestion:
>
> * @throws IllegalArgumentException if start is negative
Thanks, I think that dates from when the param was renamed.
> src/java.base/share/classes/java/lang/VirtualThread.java line 65:
>
>> 63: * system.
>> 64: */
>> 65: class VirtualThread extends Thread {
>
> Suggestion:
>
> final class VirtualThread extends Thread {
okay.
> src/java.base/share/classes/java/lang/VirtualThread.java line 94:
>
>> 92: * RUNNING -> PARKING // Thread attempts to park
>> 93: * PARKING -> PARKED // yield successful, thread is parked
>> 94: * PARKING -> PINNED // yield failed, thread is pinned
>
> Suggestion:
>
> * PARKING -> PARKED // parking successful, thread is parked
> * PARKING -> PINNED // parking failed, thread is pinned
The comment was about cont.yield, will change it to make it clearer.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - Refresh - Refresh - Merge with jdk-19+18 - Refresh - Initial push - Changes: - all: https://git.openjdk.java.net/jdk/pull/8166/files - new: https://git.openjdk.java.net/jdk/pull/8166/files/0e12c206..f53f0d4e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8166=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8166=01-02 Stats: 148912 lines in 1502 files changed: 104901 ins; 9917 del; 34094 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Wed, 13 Apr 2022 11:38:55 GMT, ExE Boss wrote:
>> Thanks - the same issue appears with `BufferedWriter`/`Writer`.
>
> The solution is to add the `private` constructor:
>
> private Reader(Object fallbackLock, Void internal) {
> // assert fallbackLock != null;
> // use InternalLock for trusted classes
> Class clazz = getClass();
> if (clazz == InputStreamReader.class
> || clazz == BufferedReader.class
> || clazz == FileReader.class
> || clazz == sun.nio.cs.StreamDecoder.class) {
> this.lock = InternalLock.newLockOr(fallbackLock);
> } else {
> this.lock = fallbackLock;
> }
> }
>
> to `Reader` (and an equivalent `private` constructor to `Writer`).
>
> ---
>
> The `protected` `Reader` constructors can then be changed to:
>
> protected Reader() {
> this(this, null);
> }
>
> and
>
> protected Reader(Object lock) {
> this(Objects.requireNonNull(lock), null);
> }
>
>
> ---
>
> Doing so will allow this change to be reverted:
> Suggestion:
>
> super(in);
> Thanks - the same issue appears with `BufferedWriter`/`Writer`.
Right. I think we can reduce this to the case where a BufferedReader is not
subclassed, and it wraps an InputStreamReader or FileReader that have not been
subclassed. Similarly the case where a BufferedWriter is not subclassed and it
wraps an OutputStreamWriter or FileWriter that have not been subclassed.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
On Wed, 13 Apr 2022 11:35:33 GMT, ExE Boss wrote: > Actually, we can’t in case `InternalLock.CAN_USE_INTERNAL_LOCK` is ever > `false` That's right, both StreamEncoder and StreamDecoder need the both cases. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v2]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/8166/files - new: https://git.openjdk.java.net/jdk/pull/8166/files/21972f45..0e12c206 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8166=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8166=00-01 Stats: 1837 lines in 112 files changed: 786 ins; 531 del; 520 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
Re: RFR: 8284161: Implementation of Virtual Threads (Preview)
On Tue, 12 Apr 2022 16:49:41 GMT, Daniel Jeliński wrote: > If it was, please update the javadoc - `NANOSECONDS.toMillis(-1)` = 0 implies > no waiting in Net.poll It's benign for the current usages but you are right, it was not intentional. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview)
On Mon, 11 Apr 2022 17:27:11 GMT, Daniel Fuchs wrote: > Not sure if that even matters - but there will be a slight change of > behaviour here if `InternalLock.CAN_USE_INTERNAL_LOCK` is ever `false`. > Instead of synchronizing on `in`, the `BufferedReader` will synchronize on > `this`. Good! We can change this so that depends on whether BufferedReader is extended and whether the given Reader is trusted. It's not clear if anyone could reliably depend on undocumented behavior like this but we have to be cautious at the same time. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284616: Implement workaround for CODETOOLS-7901986 in serviceability/jvmti/RedefineClasses
On Mon, 11 Apr 2022 06:41:54 GMT, Alan Bateman wrote: >> The tests serviceability/jvmti/RedefineClasses start failing in loom-repo >> with CNFE like "java.lang.NoClassDefFoundError: >> jdk/test/lib/compiler/InMemoryJavaCompiler" >> >> It is not a loom specific bug, it might happen in jdk/jdk also. >> >> The workaround is to build some classes explicitly. The fix was implemented >> in repo-loom in Nov 2021 and there was no such failure after the fix in the >> loom repo. >> >> Also fix inlcudes some text refactoring to make push from loom smaller. > > The issue with RedefineClassTest throwing NoClassDefFoundError: > jdk/test/lib/InMemoryJavaCompiler seems to go back to 2016 at least. We've > had issues in other areas in the past that stemmed from implicit compilation > so I assume this is what is suspected here too. > > In the loom repo, two of the existing RedefineClasss* classes have an > explicit `@compile` because the updated tests needed to be compiled with > `--enable-preview`. I would be surprised if this caused an issue but maybe it > creates an issue with concurrent test execution when tests that depend on > implicit compilation are running at the same time (in another agentvm) but > doing explicit compilation? I wonder if creating a TEST.properties with > `exclusiveAccess.dirs=.` would help this area. > @AlanBateman adding the explicit compile commands to add `--enable-preview` > is exactly what causes the problem. By compiling that individual file first > it also compiled some testlib dependencies but puts the classes in a > different place. When another class is later compiled, the compilation path > includes that destination and so compilation succeeds, but at runtime the > path is different and we get the NCDFE. This is the analysis that Alex did in > a similar issue - see his comment in JBS. Adding `--enable-preview` did not intentionally mean to introduce implicit compilation but it looks like we'll have to re-visit the ordering (as @alexmenkov suggests) in the loom repo. - PR: https://git.openjdk.java.net/jdk/pull/8170
Re: RFR: 8284161: Implementation of Virtual Threads (Preview)
On Mon, 11 Apr 2022 07:33:20 GMT, ExE Boss wrote: > Maybe it should use a `MethodHandle` fetched using `IMPL_LOOKUP` instead, in > order to avoid the runtime overhead of going through `CallerSensitive` > methods (`Class.forName` and `Constructor.newInstance`). > > It should probably also be cached. It is cached. It's also a very exceptional case that only arises when the SC API (separate JEP, not here) is used incorrectly. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview)
On Sun, 10 Apr 2022 22:14:41 GMT, Magnus Ihse Bursie wrote: > Should this not be `jdk.internal.misc`? (Also, if this is the case, maybe > there's a test missing that could have discovered this...) The exception is in an incubator module, it's just that code in java.base can't statically reference it. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview)
On Sun, 10 Apr 2022 00:40:02 GMT, ExE Boss wrote: > This is supposed to point to the `package‑summary.html` Thanks, this link is indeed wrong. Will be fixed when we refresh. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284616: Implement workaround for CODETOOLS-7901986 in serviceability/jvmti/RedefineClasses
On Fri, 8 Apr 2022 22:15:21 GMT, Leonid Mesnik wrote: > The tests serviceability/jvmti/RedefineClasses start failing in loom-repo > with CNFE like "java.lang.NoClassDefFoundError: > jdk/test/lib/compiler/InMemoryJavaCompiler" > > It is not a loom specific bug, it might happen in jdk/jdk also. > > The workaround is to build some classes explicitly. The fix was implemented > in repo-loom in Nov 2021 and there was no such failure after the fix in the > loom repo. > > Also fix inlcudes some text refactoring to make push from loom smaller. The issue with RedefineClassTest throwing NoClassDefFoundError: jdk/test/lib/InMemoryJavaCompiler seems to go back to 2016 at least. We've had issues in other areas in the past that stemmed from implicit compilation so I assume this is what is suspected here too. In the loom repo, two of the existing RedefineClasss* classes have an explicit `@compile` because the updated tests needed to be compiled with `--enable-preview`. I would be surprised if this caused an issue but maybe it creates an issue with concurrent test execution when tests that depend on implicit compilation are running at the same time (in another agentvm) but doing explicit compilation? I wonder if creating a TEST.properties with `exclusiveAccess.dirs=.` would help this area. - PR: https://git.openjdk.java.net/jdk/pull/8170
RFR: 8284161: Implementation of Virtual Threads (Preview)
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. - Commit messages: - Initial push Changes: https://git.openjdk.java.net/jdk/pull/8166/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8166=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284161 Stats: 101472 lines in 1116 files changed: 91922 ins; 4207 del; 5343 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
Re: RFR: 8282691: add jdb "-R" option for passing any argument to the launched debuggee process [v4]
On Sat, 12 Mar 2022 05:44:21 GMT, Chris Plummer wrote: >> jdb has 3 types of arguments: >> >> 1. Those that are jdb specific, such as -attach, -launch, and -listconnectors >> 2. Those that are passed to the JVM used to run jdb. These are all prefixed >> with -J, and any valid JVM argument can be passed in this manner. >> 3. Those that are passed to the debuggee JVM when it is launched, such as >> -classpath, -D, and any option that starts with -X (including -XX) >> >> The problem with the 3rd group is that (other than for -D and -X), jdb will >> only pass through arguments that it recognizes, and that list is very >> limited. When you want to launch the debuggee with an argument that jdb >> doesn't recognize, you have no choice but to launch the debuggee yourself >> (using a separate command line and using the -agentlib:jdwp argument) and >> then tell jdb to attach to the debuggee process. It's much easier when you >> can just let jdb launch the debuggee, and our nsk/jdb testing currently >> relies on this feature. >> >> This PR adds --enable-preview and --add-modules to the list of arguments >> that jdb recognizes and passes through to the debuggee JVM. These seem to be >> the two most glaring omissions, and are two that will likely be needed soon >> in order for loom nsk/jdb testing to work properly. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Updated jdb man page for -R command. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7708
Re: RFR: 8282691: add jdb "-R" option for passing any argument to the launched debuggee process [v3]
On Tue, 8 Mar 2022 19:26:41 GMT, Chris Plummer wrote: >> jdb has 3 types of arguments: >> >> 1. Those that are jdb specific, such as -attach, -launch, and -listconnectors >> 2. Those that are passed to the JVM used to run jdb. These are all prefixed >> with -J, and any valid JVM argument can be passed in this manner. >> 3. Those that are passed to the debuggee JVM when it is launched, such as >> -classpath, -D, and any option that starts with -X (including -XX) >> >> The problem with the 3rd group is that (other than for -D and -X), jdb will >> only pass through arguments that it recognizes, and that list is very >> limited. When you want to launch the debuggee with an argument that jdb >> doesn't recognize, you have no choice but to launch the debuggee yourself >> (using a separate command line and using the -agentlib:jdwp argument) and >> then tell jdb to attach to the debuggee process. It's much easier when you >> can just let jdb launch the debuggee, and our nsk/jdb testing currently >> relies on this feature. >> >> This PR adds --enable-preview and --add-modules to the list of arguments >> that jdb recognizes and passes through to the debuggee JVM. These seem to be >> the two most glaring omissions, and are two that will likely be needed soon >> in order for loom nsk/jdb testing to work properly. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Minor help message improvements. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7708
Re: RFR: 8282691: add jdb support for passing --enable-preview and --add-modules to the debuggee JVM [v2]
On Tue, 8 Mar 2022 03:37:44 GMT, Chris Plummer wrote: >> jdb has 3 types of arguments: >> >> 1. Those that are jdb specific, such as -attach, -launch, and -listconnectors >> 2. Those that are passed to the JVM used to run jdb. These are all prefixed >> with -J, and any valid JVM argument can be passed in this manner. >> 3. Those that are passed to the debuggee JVM when it is launched, such as >> -classpath, -D, and any option that starts with -X (including -XX) >> >> The problem with the 3rd group is that (other than for -D and -X), jdb will >> only pass through arguments that it recognizes, and that list is very >> limited. When you want to launch the debuggee with an argument that jdb >> doesn't recognize, you have no choice but to launch the debuggee yourself >> (using a separate command line and using the -agentlib:jdwp argument) and >> then tell jdb to attach to the debuggee process. It's much easier when you >> can just let jdb launch the debuggee, and our nsk/jdb testing currently >> relies on this feature. >> >> This PR adds --enable-preview and --add-modules to the list of arguments >> that jdb recognizes and passes through to the debuggee JVM. These seem to be >> the two most glaring omissions, and are two that will likely be needed soon >> in order for loom nsk/jdb testing to work properly. > > Chris Plummer has updated the pull request incrementally with three > additional commits since the last revision: > > - Re-add copyright change. > - Remove copyright change. > - Support -R instead of --enable-preview and --add-modules. src/jdk.jdi/share/classes/com/sun/tools/example/debug/tty/TTYResources.java line 480: > 478: "-tclient run the application in the > HotSpot(TM) Client Compiler\n" + > 479: "-tserver run the application in the > HotSpot(TM) Server Compiler\n" + > 480: "-RForward to debuggee > process\n" + The existing description start with a lowercase character so probably best to keep that consistent. Also maybe it should make it clear that the options are forwarded to the debuggee then it is launched by jdb, the option is otherwise ignored. - PR: https://git.openjdk.java.net/jdk/pull/7708
