On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman <al...@openjdk.org> 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

Reply via email to