Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview). >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I have reviewed the following portions of this change: test/common, test/gtest, test/hotspot/runtime, test/jdk/jfr, test library Feedback: - see several minor comments inline - under runtime/cds/appcds/test-classes there is an empty "TEST.properties" file; was it added by mistake? With only a few minor comments, I approve of the code reviewed by me provided that my comments will be addressed. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview). >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/lib/jdk/test/lib/thread/VThreadRunner.java line 61: > 59: /** > 60: * Run a task in a virtual thread and wait for it to terminate. > 61: * If the task completse with an exception then it is thrown by this > method. typo: completse --> completes test/lib/jdk/test/lib/thread/VThreadRunner.java line 121: > 119: /** > 120: * Run a task in a virtual thread and wait for it to terminate. > 121: * If the task completse with an exception then it is thrown by this > method. completse --> completes - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8279358: vmTestbase/nsk/jvmti/scenarios/jni_interception/JI03/ji03t003/TestDescription.java fails with usage tracker
On Fri, 29 Apr 2022 22:12:28 GMT, Chris Plummer wrote: >> The test counts calls of intercepted JNI functions, but doesn't completely >> filter out calls from other threads. >> Function isThreadExpected is used only for ExceptionOccurred function and >> the function checks only some known JFR/Graal threads. >> >> The change: >> - updates the test to count only the calls on the test thread; >> - adds verbose output. > > Also there is > [JDK-8284027](https://bugs.openjdk.java.net/browse/JDK-8284027), which also > is a failure related to unexpected threads and fails because > isThreadExpected() is not filtering all the proper threads (loom makes this > issue worse). It looks like you are trying to make the individual tests > smarter rather than fixing isThreadExpected(). I'm not saying this is the > wrong approach, but we should apply the approach to all tests that use > isThreadExpected() . @plummercj ji01t001 checks that the number of the calls >= of the expected. I thought about make the same condition here, but decided that it would be better to verify for equality, but ensure we count the calls from the test thread only - PR: https://git.openjdk.java.net/jdk/pull/8475
Re: RFR: 8279358: vmTestbase/nsk/jvmti/scenarios/jni_interception/JI03/ji03t003/TestDescription.java fails with usage tracker
On Fri, 29 Apr 2022 21:39:20 GMT, Alex Menkov wrote: > The test counts calls of intercepted JNI functions, but doesn't completely > filter out calls from other threads. > Function isThreadExpected is used only for ExceptionOccurred function and the > function checks only some known JFR/Graal threads. > > The change: > - updates the test to count only the calls on the test thread; > - adds verbose output. Also there is [JDK-8284027](https://bugs.openjdk.java.net/browse/JDK-8284027), which also is a failure related to unexpected threads and fails because isThreadExpected() is not filtering all the proper threads (loom makes this issue worse). It looks like you are trying to make the individual tests smarter rather than fixing isThreadExpected(). I'm not saying this is the wrong approach, but we should apply the approach to all tests that use isThreadExpected() . - PR: https://git.openjdk.java.net/jdk/pull/8475
Re: RFR: 8279358: vmTestbase/nsk/jvmti/scenarios/jni_interception/JI03/ji03t003/TestDescription.java fails with usage tracker
On Fri, 29 Apr 2022 21:39:20 GMT, Alex Menkov wrote: > The test counts calls of intercepted JNI functions, but doesn't completely > filter out calls from other threads. > Function isThreadExpected is used only for ExceptionOccurred function and the > function checks only some known JFR/Graal threads. > > The change: > - updates the test to count only the calls on the test thread; > - adds verbose output. What about ji01t001? Seems it can suffer from the same issues. I know I see the WARNING message in its output for unexpected intercept calls. - PR: https://git.openjdk.java.net/jdk/pull/8475
RFR: 8279358: vmTestbase/nsk/jvmti/scenarios/jni_interception/JI03/ji03t003/TestDescription.java fails with usage tracker
The test counts calls of intercepted JNI functions, but doesn't completely filter out calls from other threads. Function isThreadExpected is used only for ExceptionOccurred function and the function checks only some known JFR/Graal threads. The change: - updates the test to count only the calls on the test thread; - adds verbose output. - Commit messages: - Fix the test Changes: https://git.openjdk.java.net/jdk/pull/8475/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8475=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279358 Stats: 54 lines in 2 files changed: 39 ins; 0 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/8475.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8475/head:pull/8475 PR: https://git.openjdk.java.net/jdk/pull/8475
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 18:27:27 GMT, Mikhailo Seledtsov wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/jdk/jdk/jfr/api/consumer/TestManyRecordings.java line 57: > >> 55: int classLoaderCount = Integer.parseInt(args[0]); >> 56: int classCount = Integer.parseInt(args[1]); >> 57: for (int i = 0; i > Did you mean classLoaderCount here instead of classCount? Also, please make > sure there is a space between < and classLoaderCount. The JFR "tests" look strange. I would expect a test called TestManyRecording to start recordings, not created a lot of classes, similar to TestManyClasses. How is this related to Loom? Could this be a merge gone bad? - PR: https://git.openjdk.java.net/jdk/pull/8166
Integrated: 8284331: Add sanity check for signal handler modification warning.
On Tue, 5 Apr 2022 10:37:26 GMT, Kevin Walls wrote: > A sanity check using "jcmd VM.info" to catch the signal handler modification > warning: it should never trigger during this test. This pull request has now been integrated. Changeset: 116763cb Author:Kevin Walls URL: https://git.openjdk.java.net/jdk/commit/116763cb5d58a7316b7bada689a0fa34a7250ee7 Stats: 20 lines in 1 file changed: 19 ins; 0 del; 1 mod 8284331: Add sanity check for signal handler modification warning. Reviewed-by: dholmes, amenkov - PR: https://git.openjdk.java.net/jdk/pull/8106
Re: RFR: 8284331: Add sanity check for signal handler modification warning.
On Tue, 5 Apr 2022 10:37:26 GMT, Kevin Walls wrote: > A sanity check using "jcmd VM.info" to catch the signal handler modification > warning: it should never trigger during this test. thanks David and Alex and Chris 8-) - PR: https://git.openjdk.java.net/jdk/pull/8106
Re: RFR: 8284331: Add sanity check for signal handler modification warning.
On Tue, 5 Apr 2022 10:37:26 GMT, Kevin Walls wrote: > A sanity check using "jcmd VM.info" to catch the signal handler modification > warning: it should never trigger during this test. ok - PR: https://git.openjdk.java.net/jdk/pull/8106
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview). >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/jdk/jdk/jfr/api/consumer/TestManyClasses.java line 57: > 55: int classLoaderCount = Integer.parseInt(args[0]); > 56: int classCount = Integer.parseInt(args[1]); > 57: for (int i = 0; i 62: theClass.newInstance(); > 63: TestEvent event = new TestEvent(); > 64: event.theClass = event; Did you mean event.theClass = theClass instead ? test/jdk/jdk/jfr/api/consumer/TestManyRecordings.java line 57: > 55: int classLoaderCount = Integer.parseInt(args[0]); > 56: int classCount = Integer.parseInt(args[1]); > 57: for (int i = 0; i https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Fri, 29 Apr 2022 18:23:35 GMT, Mikhailo Seledtsov wrote: >> Alan Bateman has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e > > test/jdk/jdk/jfr/api/consumer/TestManyClasses.java line 57: > >> 55: int classLoaderCount = Integer.parseInt(args[0]); >> 56: int classCount = Integer.parseInt(args[1]); >> 57: for (int i = 0; i > Minor: Style: please insert space between < and classCount Also, should this be i < classLoaderCount ?? - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview). >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling _Unimplemented_) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Extent Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/hotspot/jtreg/runtime/vthread/StackChunkClassLoaderTest.java line 2: > 1: /* > 2: * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. Please update copyright year. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v4]
On Fri, 29 Apr 2022 10:56:49 GMT, Kim Barrett wrote: >> The current approach limits the knowledge of non-strong ref types to >> `instanceRefKlass` file. > > The `_reference_type` used to be initialized correctly in most cases, but > needed fixing up for a few cases during bootstrapping. With this change it is > *never* initialized correctly for Reference subtypes and always needs an > initialization kludge for them. That's not an improvement. `_reference_type` always gets the correct value after the constructor is run. The member initializer list just follows the convention of having all fields set. One could move this field inside the constructor body to ensure this field is set only once. - PR: https://git.openjdk.java.net/jdk/pull/8332
Re: RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v4]
On Thu, 28 Apr 2022 20:06:47 GMT, Albert Mingkun Yang wrote: >> src/hotspot/share/oops/instanceKlass.cpp line 497: >> >>> 495: _nest_host_index(0), >>> 496: _init_state(allocated), >>> 497: _reference_type(REF_NONE), >> >> This is initializing `_reference_type` to the wrong value for a >> `InstanceRefKlass` object, which then needs to reset it in the derived >> constructor. Why not get the reference type from the parser? The >> (currently file-scoped static) determine_reference_type function in >> instanceRefKlass.cpp doesn't have any dependency on the klass object being >> constructed, just the parser. > > The current approach limits the knowledge of non-strong ref types to > `instanceRefKlass` file. The `_reference_type` used to be initialized correctly in most cases, but needed fixing up for a few cases during bootstrapping. With this change it is *never* initialized correctly for Reference subtypes and always needs an initialization kludge for them. That's not an improvement. - PR: https://git.openjdk.java.net/jdk/pull/8332
Re: RFR: 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment [v2]
On Thu, 28 Apr 2022 13:18:59 GMT, David Holmes wrote:
>> Johannes Bechberger has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Use Thread::current_or_null_safe() and add comments
>
> src/hotspot/share/prims/forte.cpp line 571:
>
>> 569: Thread* raw_thread = Thread::current_or_null_safe();
>> 570:
>> 571: if (trace->env_id == NULL || raw_thread == NULL ||
>> !raw_thread->is_Java_thread() || ((JavaThread*)raw_thread)->is_exiting()) {
>
> use `rawThread->as_JavaThread()` not a plain cast.
this method does not exist
-
PR: https://git.openjdk.java.net/jdk/pull/8446
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v5]
On Fri, 29 Apr 2022 08:45:42 GMT, Pavel Rappo wrote: > Good catch. Sorry I missed it. This occurs in all `java/lang/ref` files. I've created a PR; feel free to review it at your convenience. - PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v5]
On Thu, 28 Apr 2022 19:06:04 GMT, Mandy Chung wrote:
>> src/java.base/share/classes/java/lang/ref/PhantomReference.java line 48:
>>
>>> 46: * The {@link #refersTo(Object) refersTo} method can be used to test
>>> 47: * whether some object is the referent of a phantom reference.
>>> 48: * @param the type of the referent
>>
>> Shouldn't there be a space after `@param` ?
>
> Good catch. Sorry I missed it. This occurs in all `java/lang/ref` files.
> Shouldn't there be a space after `@param` ?
> Good catch. Sorry I missed it. This occurs in all `java/lang/ref` files.
I built the API documentation after this PR has been integrated and the result
was okay. I saw this output in every such case:
Type Parameters:
T - the type of the referent
javadoc is quite robust. However, for some IDEs such missing whitespace seems
significant. Not only do they highlight the `@param` tag, but the type
parameter information is missing from the rendered output.
Although it's not critical, we should fix it; I have filed JDK-8285890.
-
PR: https://git.openjdk.java.net/jdk/pull/8410
Re: RFR: 8284331: Add sanity check for signal handler modification warning.
On Fri, 29 Apr 2022 03:15:09 GMT, Chris Plummer wrote: > How does this relate the failure in JDK-8285647? Is this just meant to detect > that failure, but a proper fix is still needed for it? ..it's not directly related - I had this test addition in progress already, as an addition to JDK-8283337 which fixes the modification handler being broken by a previous change (the warning was firing all the time, after JDK-8279124 and before JDK-8283337). So we should have a sanity check (this PR) which fails if the warning starts firing unnecessarily. Logged JDK-8285792 to signal that we need some more cleanup here, including the news that I think JDK-8285647 shows that we don't consistently ignore the crash_handler. - PR: https://git.openjdk.java.net/jdk/pull/8106
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e I'm sorry for the noise with my comments. I'll continue to discuss it privately unless there is something really important. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/hotspot/jtreg/serviceability/jvmti/events/Exception/exception01/libexception01.cpp line 178: > 176:ex.c_cls, ex.c_name, ex.c_sig, > 177:(jint)(ex.c_loc >> 32), (jint)ex.c_loc); > 178: result = STATUS_FAILED; Unaligned lines in the range: 172-177. There are some non-uinified unneeded spaces at lines 172,175. test/hotspot/jtreg/serviceability/jvmti/events/Exception/exception01/libexception01.cpp line 287: > 285: } > 286: > 287: eventsCount = 0; Counters are not protected with locks. I'm not sure it is a real problem here but would be better to double-check. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
>> JDK version to target.
>>
>> We will refresh this PR periodically to pick up changes and fixes from the
>> loom repo.
>>
>> Most of the new mechanisms in the HotSpot VM are disabled by default and
>> require running with `--enable-preview` to enable.
>>
>> The patch has support for x64 and aarch64 on the usual operating systems
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for
>> zero and some of the other ports. Additional ports can be contributed via
>> PRs against the fibers branch in the loom repo.
>>
>> There are changes in many areas. To reduce notifications/mails, the labels
>> have been trimmed down for now to hotspot, serviceability and core-libs.
>> We'll add the complete set of labels when the PR is further along.
>>
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from
>> Doug Lea's CVS. These changes will probably be proposed and integrated in
>> advance of this PR.
>>
>> The changes include some non-exposed and low-level infrastructure to support
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to
>> make life a bit easier and avoid having to separate VM changes and juggle
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
test/hotspot/jtreg/serviceability/jvmti/events/ClassPrepare/classprep01/libclassprep01.cpp
line 59:
> 57: static jvmtiEventCallbacks callbacks;
> 58: static jint result = PASSED;
> 59: static volatile size_t eventsCount = 0; // TODO these 2 vars mofified
> from different threads in getReady/check. What to DO???
I'd suggest to use RawMonitorLocker with event_lock or counter_lock to protect
updates of these counters.
You can remove this comment then.
test/hotspot/jtreg/serviceability/jvmti/events/ClassPrepare/classprep01/libclassprep01.cpp
line 201:
> 199: LOG("\n");
> 200:
> 201:
Too many empty lines. It might make sense to do a common cleanup with all edits
like this.
test/hotspot/jtreg/serviceability/jvmti/events/ClassPrepare/classprep01/libclassprep01.cpp
line 258:
> 256: return JNI_VERSION_1_8;
> 257: }
> 258: #endif
Empty line is missed => common cleanup.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote: >> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which >> JDK version to target. >> >> We will refresh this PR periodically to pick up changes and fixes from the >> loom repo. >> >> Most of the new mechanisms in the HotSpot VM are disabled by default and >> require running with `--enable-preview` to enable. >> >> The patch has support for x64 and aarch64 on the usual operating systems >> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for >> zero and some of the other ports. Additional ports can be contributed via >> PRs against the fibers branch in the loom repo. >> >> There are changes in many areas. To reduce notifications/mails, the labels >> have been trimmed down for now to hotspot, serviceability and core-libs. >> We'll add the complete set of labels when the PR is further along. >> >> The changes include a refresh of java.util.concurrent and ForkJoinPool from >> Doug Lea's CVS. These changes will probably be proposed and integrated in >> advance of this PR. >> >> The changes include some non-exposed and low-level infrastructure to support >> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to >> make life a bit easier and avoid having to separate VM changes and juggle >> branches at this time. > > Alan Bateman has updated the pull request incrementally with one additional > commit since the last revision: > > Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e test/hotspot/jtreg/serviceability/jvmti/events/ClassLoad/classload01/libclassload01.cpp line 139: > 137: primClsEvents[i]++; > 138: LOG( > 139: "TEST FAILED: JVMTI_EVENT_CLASS_LOAD event received for\n" Combine 138+138. I won't comment this more in hope the same will be fixed in all tests. test/hotspot/jtreg/serviceability/jvmti/events/ClassLoad/classload01/libclassload01.cpp line 177: > 175: return JNI_VERSION_1_8; > 176: } > 177: #endif One empty line would be nice to add after 177. - PR: https://git.openjdk.java.net/jdk/pull/8166
Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]
On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman wrote:
>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which
>> JDK version to target.
>>
>> We will refresh this PR periodically to pick up changes and fixes from the
>> loom repo.
>>
>> Most of the new mechanisms in the HotSpot VM are disabled by default and
>> require running with `--enable-preview` to enable.
>>
>> The patch has support for x64 and aarch64 on the usual operating systems
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for
>> zero and some of the other ports. Additional ports can be contributed via
>> PRs against the fibers branch in the loom repo.
>>
>> There are changes in many areas. To reduce notifications/mails, the labels
>> have been trimmed down for now to hotspot, serviceability and core-libs.
>> We'll add the complete set of labels when the PR is further along.
>>
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from
>> Doug Lea's CVS. These changes will probably be proposed and integrated in
>> advance of this PR.
>>
>> The changes include some non-exposed and low-level infrastructure to support
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to
>> make life a bit easier and avoid having to separate VM changes and juggle
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 139:
> 137:thr_info.name,(jni->IsVirtualThread(thread) ==
> JNI_TRUE) ? "virtual" : "kernel",
> 138: (thr_info.is_daemon == JNI_TRUE) ? "deamon" : "user");
> 139: }
I'd suggest to add locals (their names are up to you):
const char* thr_virtual_tag = jni->IsVirtualThread(thread) == JNI_TRUE ?
"virtual" : "kernel";
const char* thr_daemon_tag == JNI_TRUE) ? "deamon" : "user";
It is better to place togeter lines: 129+130.
Line 137 is not aligned, and there are many unneeded spaces.
The '()' around conditions are not needed. At least, I do not see them
increasing readability.
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 167:
> 165: result = checkStatus = STATUS_FAILED;
> 166: LOG(
> 167: "TEST FAILED: Breakpoint event with unexpected class
> signature:\n"
Combine lines 166+167.
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 187:
> 185: jboolean isVirtual = jni->IsVirtualThread(thread);
> 186: if (isVirtual != METHODS_ATTRS[i]) {
> 187: LOG("TEST FAILED: IsVirtualThread check failed with unexpected
> result %d when expected is %d\n", isVirtual, METHODS_ATTRS[i]);
Line 187 is too long and can be splitted.
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 231:
> 229: result = STATUS_FAILED;
> 230: LOG(
> 231: "TEST FAILED: wrong number of Breakpoint events\n"
Combine 230+231.
test/hotspot/jtreg/serviceability/jvmti/events/Breakpoint/breakpoint01/libbreakpoint01.cpp
line 307:
> 305: if (agent_lock == NULL) {
> 306: return JNI_ERR;
> 307: }
Better to also use same style with curly brackets in fragments: 293, 296, 299.
-
PR: https://git.openjdk.java.net/jdk/pull/8166
