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