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" CSR has been approved. I still need 2 reviewers since this PR was reworked after the initial 2 reviews. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/8780
Integrated: 8287695: Use String.contains() instead of String.indexOf() in jdk.hotspot.agent
On Wed, 1 Jun 2022 07:52:50 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in jdk.hotspot.agent still uses old approach with `String.indexOf` > to check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. This pull request has now been integrated. Changeset: 26048ea2 Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/26048ea21e0da6505d8452bd33a4d37b1bd5ce74 Stats: 19 lines in 4 files changed: 0 ins; 7 del; 12 mod 8287695: Use String.contains() instead of String.indexOf() in jdk.hotspot.agent Reviewed-by: cjplummer - PR: https://git.openjdk.java.net/jdk/pull/8966
Re: RFR: 8287695: Use String.contains() instead of String.indexOf() in jdk.hotspot.agent
On Wed, 1 Jun 2022 07:52:50 GMT, Andrey Turbanov wrote: > `String.contains` was introduced in Java 5. > Some code in jdk.hotspot.agent still uses old approach with `String.indexOf` > to check if String contains specified substring. > I propose to migrate such usages. Makes code shorter and easier to read. All green. Thank you for review! - PR: https://git.openjdk.java.net/jdk/pull/8966
Re: RFR: 8287695: Use String.contains() instead of String.indexOf() in jdk.hotspot.agent
On Thu, 2 Jun 2022 09:57:10 GMT, Andrey Turbanov wrote: > > Please make sure you run the SA tests. > > This one, `test/hotspot/jtreg/serviceability` right? You can just do the `test/hotspot/jtreg/serviceability/sa` subdirectory, but there is also `test/jdk/sun/tools/jhsdb` - PR: https://git.openjdk.java.net/jdk/pull/8966
Integrated: 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 This pull request has now been integrated. Changeset: 3cfd38ca Author:Aleksey Shipilev URL: https://git.openjdk.java.net/jdk/commit/3cfd38caf10c18f71c0fc8c9a09c0d1179373ce7 Stats: 138 lines in 69 files changed: 69 ins; 69 del; 0 mod 8287726: Fix JVMTI tests with "requires vm.continuations" after JDK-8287496 Reviewed-by: alanb, rehn, lmesnik - 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 Thank you! - PR: https://git.openjdk.java.net/jdk/pull/8990
Re: RFR: 8287663 Add a regression test for JDK-8287073
On Thu, 2 Jun 2022 14:32:28 GMT, Severin Gehwolf wrote: > This adds a regression test for a recent fix (JDK-8287073). I've restructured > the linux specific JDK code to call a separate static function to enable this > test. It'll help future tests too. > > Testing: > - [x] Container tests continue to pass + GHA > - [x] New regression test fails prior the code fix of JDK-8287073 and passes > with it @jerboaa Thanks for taking care of the test! I'm not a reviewer, but FWIW the test looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/8993
Re: RFR: 8287726: Fix JVMTI tests with "requires vm.continuations" after JDK-8287496
On Thu, 2 Jun 2022 14:38:31 GMT, Aleksey Shipilev wrote: > Trivial, or? I would like to have this integrated sooner to clean up our > testing. Ship it under trivial, thanks. - 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 Trivial, or? I would like to have this integrated sooner to clean up our testing. - 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 Marked as reviewed by lmesnik (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8990
RFR: 8287663 Add a regression test for JDK-8287073
This adds a regression test for a recent fix (JDK-8287073). I've restructured the linux specific JDK code to call a separate static function to enable this test. It'll help future tests too. Testing: - [x] Container tests continue to pass + GHA - [x] New regression test fails prior the code fix of JDK-8287073 and passes with it - Commit messages: - 8287663 Add a regression test for JDK-8287073 Changes: https://git.openjdk.java.net/jdk/pull/8993/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8993=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287663 Stats: 36 lines in 2 files changed: 35 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8993.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8993/head:pull/8993 PR: https://git.openjdk.java.net/jdk/pull/8993
RFR: 8287281: adjust guarantee in Handshake::execute for the case of target thread being current
Please review this PR for fixing JDK-8287281. I chose a different solution than the one suggested. Looking at all callers of `Handshake::execute`, it seems that only one depends on `target == current`. The rest special case that by calling `is_handshake_safe_for` and `do_thread` directly. I converted the only instance of `Handshake::execute` with `target == current` to just directly call `do_thread`. Furthermore we now explicitly check for this case in `Handshake::execute` with an assert and document that this should not be done. Finally: Should `VirtualThreadGetThreadClosure` and its `do_thread()` body be inlined instead? We can do this in this PR, imho, but I'm hoping to get some input on this. Currently running tier1-5 to check if I'm missing something. - Commit messages: - Disallow handshaking with self Changes: https://git.openjdk.java.net/jdk/pull/8992/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8992=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287281 Stats: 8 lines in 3 files changed: 7 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8992.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8992/head:pull/8992 PR: https://git.openjdk.java.net/jdk/pull/8992
Re: RFR: 8287281: adjust guarantee in Handshake::execute for the case of target thread being current
On Thu, 2 Jun 2022 13:47:23 GMT, Johan Sjölén wrote: > Please review this PR for fixing JDK-8287281. > > I chose a different solution than the one suggested. Looking at all callers > of `Handshake::execute`, it seems that only one depends on `target == > current`. The rest special case that by calling `is_handshake_safe_for` and > `do_thread` directly. I converted the only instance of `Handshake::execute` > with `target == current` to just directly call `do_thread`. > > Furthermore we now explicitly check for this case in `Handshake::execute` > with an assert and document that this should not be done. > > Finally: Should `VirtualThreadGetThreadClosure` and its `do_thread()` body > be inlined instead? We can do this in this PR, imho, but I'm hoping to get > some input on this. > > > Currently running tier1-5 to check if I'm missing something. I agree, thanks for fixing. - Marked as reviewed by rehn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8992
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 rehn (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 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 11:05:30 GMT, Alan Bateman wrote: > 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. Yes, in separate PR. In this PR, I want to deal with JVMTI tests specifically. We can take JVMTI tests off the problemlist exactly because of `@requires vm.continuations` added in this PR. - 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
Re: RFR: 8287695: Use String.contains() instead of String.indexOf() in jdk.hotspot.agent
On Wed, 1 Jun 2022 20:59:41 GMT, Chris Plummer wrote: > Please make sure you run the SA tests. This one, `test/hotspot/jtreg/serviceability` right? - PR: https://git.openjdk.java.net/jdk/pull/8966
RFR: 8287726: Fix JVMTI tests with "requires vm.continuations" after JDK-8287496
[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 x86_32 problemlist exclusions, which serves a proof of concept that [JDK-8287496](https://bugs.openjdk.java.net/browse/JDK-8287496) indeed works. Additional testing: - [x] Linux x86_32 fastdebug, `serviceability/jvmti` works, many tests skipped - [ ] Linux x86_64 fastdebug, `serviceability/jvmti` still works, with all current tests executing - Commit messages: - Only trim down JVMTI tests from x86_32 problemlists - Fix Changes: https://git.openjdk.java.net/jdk/pull/8990/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8990=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287726 Stats: 138 lines in 69 files changed: 69 ins; 69 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8990.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8990/head:pull/8990 PR: https://git.openjdk.java.net/jdk/pull/8990
Re: RFR: 8287496: Alternative virtual thread implementation that maps to OS thread [v3]
On Wed, 1 Jun 2022 06:04:14 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. > > 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 > 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. See #8990. - PR: https://git.openjdk.java.net/jdk/pull/8939
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
