RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning
Patricio Chilano Mateo
pchilanomate at openjdk.org
Wed Nov 6 17:40:05 UTC 2024
On Mon, 28 Oct 2024 01:13:05 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> This is the implementation of JEP 491: Synchronize Virtual Threads without Pinning. See [JEP 491](https://bugs.openjdk.org/browse/JDK-8337395) for further details.
>>
>> In order to make the code review easier the changes have been split into the following initial 4 commits:
>>
>> - Changes to allow unmounting a virtual thread that is currently holding monitors.
>> - Changes to allow unmounting a virtual thread blocked on synchronized trying to acquire the monitor.
>> - Changes to allow unmounting a virtual thread blocked in `Object.wait()` and its timed-wait variants.
>> - Changes to tests, JFR pinned event, and other changes in the JDK libraries.
>>
>> The changes fix pinning issues for all 4 ports that currently implement continuations: x64, aarch64, riscv and ppc. Note: ppc changes were added recently and stand in its own commit after the initial ones.
>>
>> The changes fix pinning issues when using `LM_LIGHTWEIGHT`, i.e. the default locking mode, (and `LM_MONITOR` which comes for free), but not when using `LM_LEGACY` mode. Note that the `LockingMode` flag has already been deprecated ([JDK-8334299](https://bugs.openjdk.org/browse/JDK-8334299)), with the intention to remove `LM_LEGACY` code in future releases.
>>
>>
>> ## Summary of changes
>>
>> ### Unmount virtual thread while holding monitors
>>
>> As stated in the JEP, currently when a virtual thread enters a synchronized method or block, the JVM records the virtual thread's carrier platform thread as holding the monitor, not the virtual thread itself. This prevents the virtual thread from being unmounted from its carrier, as ownership information would otherwise go wrong. In order to fix this limitation we will do two things:
>>
>> - We copy the oops stored in the LockStack of the carrier to the stackChunk when freezing (and clear the LockStack). We copy the oops back to the LockStack of the next carrier when thawing for the first time (and clear them from the stackChunk). Note that we currently assume carriers don't hold monitors while mounting virtual threads.
>>
>> - For inflated monitors we now record the `java.lang.Thread.tid` of the owner in the ObjectMonitor's `_owner` field instead of a JavaThread*. This allows us to tie the owner of the monitor to a `java.lang.Thread` instance, rather than to a JavaThread which is only created per platform thread. The tid is already a 64 bit field so we can ignore issues of the counter wrapping around.
>>
>> #### General notes about this part:
>>
>> - Since virtual th...
>
> src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp line 2382:
>
>> 2380: __ bind(after_transition);
>> 2381:
>> 2382: if (LockingMode != LM_LEGACY && method->is_object_wait0()) {
>
> It bothers me that we have to add a check for a specific native method in this code (notwithstanding there are already some checks in relation to hashCode). As a follow up I wonder if we can deal with wait-preemption by rewriting the Java code, instead of special casing the wait0 native code?
Not sure. We would have to return from wait0 and immediately clear the physical stack from the frames just copied without safepoint polls in the middle. Otherwise if someone walks the thread's stack it will find the frames appearing twice: in the physical stack and in the heap.
> The "waiting list" here is just a list of virtual threads that need unparking by the Unblocker thread - right?
>
Yes.
> src/hotspot/share/classfile/javaClasses.cpp line 2086:
>
>> 2084: jboolean vthread_on_list = Atomic::load(addr);
>> 2085: if (!vthread_on_list) {
>> 2086: vthread_on_list = Atomic::cmpxchg(addr, (jboolean)JNI_FALSE, (jboolean)JNI_TRUE);
>
> It is not clear who the racing participants are here. How can the same thread be being placed on the list from two different actions?
The same example mentioned above, with a different timing, could result in two threads trying to add the same virtual thread to the list at the same time.
> src/hotspot/share/code/nmethod.cpp line 711:
>
>> 709: // handle the case of an anchor explicitly set in continuation code that doesn't have a callee
>> 710: JavaThread* thread = reg_map->thread();
>> 711: if ((thread->has_last_Java_frame() && fr.sp() == thread->last_Java_sp()) JVMTI_ONLY(|| (method()->is_continuation_enter_intrinsic() && thread->on_monitor_waited_event()))) {
>
> Suggestion:
>
> if ((thread->has_last_Java_frame() && fr.sp() == thread->last_Java_sp())
> JVMTI_ONLY(|| (method()->is_continuation_enter_intrinsic() && thread->on_monitor_waited_event()))) {
Fixed.
> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 889:
>
>> 887: return f.is_native_frame() ? recurse_freeze_native_frame(f, caller) : recurse_freeze_stub_frame(f, caller);
>> 888: } else {
>> 889: // frame can't be freezed. Most likely the call_stub or upcall_stub
>
> Suggestion:
>
> // Frame can't be frozen. Most likely the call_stub or upcall_stub
Fixed.
> src/hotspot/share/runtime/javaThread.hpp line 165:
>
>> 163: // ID used as owner for inflated monitors. Same as the j.l.Thread.tid of the
>> 164: // current _vthread object, except during creation of the primordial and JNI
>> 165: // attached thread cases where this field can have a temporal value.
>
> Suggestion:
>
> // attached thread cases where this field can have a temporary value.
>
> Presumably this is for when the attaching thread is executing the Thread constructor?
Exactly.
> src/hotspot/share/runtime/javaThread.hpp line 166:
>
>> 164: // current _vthread object, except during creation of the primordial and JNI
>> 165: // attached thread cases where this field can have a temporary value. Also,
>> 166: // calls to VirtualThread.switchToCarrierThread will temporary change _vthread
>
> s/temporary change/temporarily change/
Fixed.
> src/hotspot/share/runtime/objectMonitor.cpp line 132:
>
>> 130:
>> 131: // -----------------------------------------------------------------------------
>> 132: // Theory of operations -- Monitors lists, thread residency, etc:
>
> This comment block needs updating now owner is not a JavaThread*, and to account for vthread usage
Updated comment.
> src/hotspot/share/runtime/objectMonitor.cpp line 1140:
>
>> 1138: }
>> 1139:
>> 1140: bool ObjectMonitor::resume_operation(JavaThread* current, ObjectWaiter* node, ContinuationWrapper& cont) {
>
> Explanatory comment would be good - thanks.
Added comment.
> src/hotspot/share/runtime/objectMonitor.cpp line 1532:
>
>> 1530: } else if (java_lang_VirtualThread::set_onWaitingList(vthread, vthread_cxq_head())) {
>> 1531: // Virtual thread case.
>> 1532: Trigger->unpark();
>
> So ignoring for the moment that I can't see how `set_onWaitingList` could return false here, the check is just an optimisation to reduce the number of unparks issued i.e. only unpark if the list has changed?
Right.
> src/hotspot/share/runtime/objectMonitor.cpp line 1673:
>
>> 1671:
>> 1672: ContinuationEntry* ce = current->last_continuation();
>> 1673: if (interruptible && ce != nullptr && ce->is_virtual_thread()) {
>
> So IIUC this use of `interruptible` would be explained as follows:
>
> // Some calls to wait() occur in contexts that still have to pin a vthread to its carrier.
> // All such contexts perform non-interruptible waits, so by checking `interruptible` we know
> // this is a regular Object.wait call.
Yes, although the non-interruptible call is coming from ObjectLocker, which already has the NoPreemptMark, so I removed this check.
> src/hotspot/share/runtime/objectMonitor.cpp line 1706:
>
>> 1704: // on _WaitSetLock so it's not profitable to reduce the length of the
>> 1705: // critical section.
>> 1706:
>
> Please restore the blank line, else it looks like the comment block pertains to the `wait_reenter_begin`, but it doesn't.
Restored.
> src/hotspot/share/runtime/objectMonitor.cpp line 2028:
>
>> 2026: // First time we run after being preempted on Object.wait().
>> 2027: // Check if we were interrupted or the wait timed-out, and in
>> 2028: // that case remove ourselves from the _WaitSet queue.
>
> I'm not sure how to interpret this comment block - is this really two sentences because the first is not actually a sentence. Also unclear what "run" and "First time" relate to.
This vthread was unmounted on the call to `Object.wait`. Now it is mounted and "running" again, and we need to check which case it is in: notified, interrupted or timed-out. "First time" means it is the first time it's running after the original unmount on `Object.wait`. This is because once we are on the monitor reentry phase, the virtual thread can be potentially unmounted and mounted many times until it successfully acquires the monitor. Not sure how to rewrite the comment to make it clearer.
> src/hotspot/share/runtime/objectMonitor.cpp line 2054:
>
>> 2052: // Mark that we are at reenter so that we don't call this method again.
>> 2053: node->_at_reenter = true;
>> 2054: assert(!has_owner(current), "invariant");
>
> The position of this assert seems odd as it seems to be something that should hold at entry to this method.
Ok, I moved it to the beginning of resume_operation.
> src/hotspot/share/runtime/objectMonitor.hpp line 47:
>
>> 45: // ParkEvent instead. Beware, however, that the JVMTI code
>> 46: // knows about ObjectWaiters, so we'll have to reconcile that code.
>> 47: // See next_waiter(), first_waiter(), etc.
>
> This to-do is likely no longer relevant with the current changes.
Removed.
> src/hotspot/share/runtime/objectMonitor.hpp line 207:
>
>> 205:
>> 206: static void Initialize();
>> 207: static void Initialize2();
>
> Please add comment why this needs to be deferred - and till after what?
Added comment.
> src/hotspot/share/runtime/objectMonitor.hpp line 288:
>
>> 286: // Returns true if this OM has an owner, false otherwise.
>> 287: bool has_owner() const;
>> 288: int64_t owner() const; // Returns null if DEFLATER_MARKER is observed.
>
> null is not an int64_t value.
Changed to NO_OWNER.
> src/hotspot/share/runtime/objectMonitor.hpp line 292:
>
>> 290:
>> 291: static int64_t owner_for(JavaThread* thread);
>> 292: static int64_t owner_for_oop(oop vthread);
>
> Some comments describing this API would be good. I'm struggling a bit with the "owner for" terminology. I think `owner_from` would be better. And can't these just overload rather than using different names?
I changed them to `owner_from`. I added a comment referring to the return value as tid, and then I used this tid name in some other comments. Maybe this methods should be called `tid_from()`? Alternatively we could use the term owner id instead, and these would be `owner_id_from()`. In theory, this tid term or owner id (or whatever other name) does not need to be related to `j.l.Thread.tid`, it just happens that that's what we are using as the actual value for this id.
> src/hotspot/share/runtime/objectMonitor.hpp line 299:
>
>> 297: // Simply set _owner field to new_value; current value must match old_value.
>> 298: void set_owner_from_raw(int64_t old_value, int64_t new_value);
>> 299: // Same as above but uses tid of current as new value.
>
> By `tid` here (and elsewhere) you actually mean `thread->threadObj()->thread_id()` - right?
It is `thread->vthread()->thread_id()` but it will match `thread->threadObj()->thread_id()` when there is no virtual thread mounted. But we cache it in thread->_lockd_id so we retrieve it from there. I think we should probably change the name of _lock_id.
> src/hotspot/share/runtime/objectMonitor.hpp line 302:
>
>> 300: // Simply set _owner field to new_value; current value must match old_value.
>> 301: void set_owner_from_raw(int64_t old_value, int64_t new_value);
>> 302: void set_owner_from(int64_t old_value, JavaThread* current);
>
> Again some comments describing API would good. The old API had vague names like old_value and new_value because of the different forms the owner value could take. Now it is always a thread-id we can do better I think. The distinction between the raw and non-raw forms is unclear and the latter is not covered by the initial comment.
I added a comment. How about s/old_value/old_tid and s/new_value/new_tid?
> src/hotspot/share/runtime/objectMonitor.hpp line 302:
>
>> 300: void set_owner_from(int64_t old_value, JavaThread* current);
>> 301: // Set _owner field to tid of current thread; current value must be ANONYMOUS_OWNER.
>> 302: void set_owner_from_BasicLock(JavaThread* current);
>
> Shouldn't tid there be the basicLock?
So the value stored in _owner has to be ANONYMOUS_OWNER. We cannot store the BasicLock* in there as before since it can clash with some other thread's tid. We store it in the new field _stack_locker instead.
> src/hotspot/share/runtime/objectMonitor.hpp line 303:
>
>> 301: void set_owner_from_raw(int64_t old_value, int64_t new_value);
>> 302: void set_owner_from(int64_t old_value, JavaThread* current);
>> 303: // Simply set _owner field to current; current value must match basic_lock_p.
>
> Comment is no longer accurate
Fixed.
> src/hotspot/share/runtime/objectMonitor.hpp line 309:
>
>> 307: // _owner field. Returns the prior value of the _owner field.
>> 308: int64_t try_set_owner_from_raw(int64_t old_value, int64_t new_value);
>> 309: int64_t try_set_owner_from(int64_t old_value, JavaThread* current);
>
> Similar to set_owner* need better comments describing API.
Added similar comment.
> src/hotspot/share/runtime/objectMonitor.hpp line 311:
>
>> 309: int64_t try_set_owner_from(int64_t old_value, JavaThread* current);
>> 310:
>> 311: bool is_succesor(JavaThread* thread);
>
> I think `has_successor` is more appropriate here as it is not the monitor that is the successor.
Right, changed.
> src/hotspot/share/runtime/objectMonitor.hpp line 312:
>
>> 310: void set_successor(JavaThread* thread);
>> 311: void set_successor(oop vthread);
>> 312: void clear_successor();
>
> Needs descriptive comments, or at least a preceding comment explaining what a "successor" is.
Added comment.
> src/hotspot/share/runtime/objectMonitor.hpp line 315:
>
>> 313: void set_succesor(oop vthread);
>> 314: void clear_succesor();
>> 315: bool has_succesor();
>
> Sorry but `successor` has two `s` before `or`.
Fixed.
> src/hotspot/share/runtime/objectMonitor.hpp line 317:
>
>> 315: bool has_succesor();
>> 316:
>> 317: bool is_owner(JavaThread* thread) const { return owner() == owner_for(thread); }
>
> Again `has_owner` seems more appropriate
Yes, changed.
> src/hotspot/share/runtime/objectMonitor.hpp line 323:
>
>> 321: }
>> 322:
>> 323: bool is_owner_anonymous() const { return owner_raw() == ANONYMOUS_OWNER; }
>
> Again I struggle with the pre-existing `is_owner` formulation here. The target of the expression is a monitor and we are asking if the monitor has an anonymous owner.
I changed it to `has_owner_anonymous`.
> src/hotspot/share/runtime/objectMonitor.hpp line 333:
>
>> 331: bool is_stack_locker(JavaThread* current);
>> 332: BasicLock* stack_locker() const;
>> 333: void set_stack_locker(BasicLock* locker);
>
> Again `is` versus `has`, plus some general comments describing the API.
Fixed and added comments.
> src/hotspot/share/runtime/objectMonitor.hpp line 334:
>
>> 332:
>> 333: // Returns true if BasicLock* stored in _stack_locker
>> 334: // points to current's stack, false othwerwise.
>
> Suggestion:
>
> // points to current's stack, false otherwise.
Fixed.
> src/hotspot/share/runtime/objectMonitor.hpp line 349:
>
>> 347: ObjectWaiter* first_waiter() { return _WaitSet; }
>> 348: ObjectWaiter* next_waiter(ObjectWaiter* o) { return o->_next; }
>> 349: JavaThread* thread_of_waiter(ObjectWaiter* o) { return o->_thread; }
>
> This no longer looks correct if the waiter is a vthread. ??
It is, we still increment _waiters for the vthread case.
> src/hotspot/share/runtime/objectMonitor.inline.hpp line 110:
>
>> 108: }
>> 109:
>> 110: // Returns null if DEFLATER_MARKER is observed.
>
> Comment needs updating
Updated.
> src/hotspot/share/runtime/objectMonitor.inline.hpp line 130:
>
>> 128: // Returns true if owner field == DEFLATER_MARKER and false otherwise.
>> 129: // This accessor is called when we really need to know if the owner
>> 130: // field == DEFLATER_MARKER and any non-null value won't do the trick.
>
> Comment needs updating
Updated. Removed the second sentence, seemed redundant.
> src/hotspot/share/runtime/synchronizer.cpp line 670:
>
>> 668: // Top native frames in the stack will not be seen if we attempt
>> 669: // preemption, since we start walking from the last Java anchor.
>> 670: NoPreemptMark npm(current);
>
> Don't we still pin for JNI monitor usage?
Only when facing contention on this call. But once we have the monitor we don't.
> src/hotspot/share/runtime/synchronizer.hpp line 172:
>
>> 170:
>> 171: // Iterate ObjectMonitors where the owner is thread; this does NOT include
>> 172: // ObjectMonitors where owner is set to a stack lock address in thread.
>
> Comment needs updating
Updated.
> src/hotspot/share/runtime/threadIdentifier.cpp line 30:
>
>> 28:
>> 29: // starting at 3, excluding reserved values defined in ObjectMonitor.hpp
>> 30: static const int64_t INITIAL_TID = 3;
>
> Can we express this in terms of those reserved values, or are they inaccessible?
Yes, we could define a new public constant `static const int64_t FIRST_AVAILABLE_TID = 3` (or some similar name) and use it here:
diff --git a/src/hotspot/share/runtime/threadIdentifier.cpp b/src/hotspot/share/runtime/threadIdentifier.cpp
index 60d6a990779..710c3141768 100644
--- a/src/hotspot/share/runtime/threadIdentifier.cpp
+++ b/src/hotspot/share/runtime/threadIdentifier.cpp
@@ -24,15 +24,15 @@
#include "precompiled.hpp"
#include "runtime/atomic.hpp"
+#include "runtime/objectMonitor.hpp"
#include "runtime/threadIdentifier.hpp"
-// starting at 3, excluding reserved values defined in ObjectMonitor.hpp
-static const int64_t INITIAL_TID = 3;
-static volatile int64_t next_thread_id = INITIAL_TID;
+// excluding reserved values defined in ObjectMonitor.hpp
+static volatile int64_t next_thread_id = ObjectMonitor::FIRST_AVAILABLE_TID;
#ifdef ASSERT
int64_t ThreadIdentifier::initial() {
- return INITIAL_TID;
+ return ObjectMonitor::FIRST_AVAILABLE_TID;
}
#endif
Or maybe define it as MAX_RESERVED_TID instead, and here we would add one to it.
> src/hotspot/share/services/threadService.cpp line 467:
>
>> 465: if (waitingToLockMonitor->has_owner()) {
>> 466: currentThread = Threads::owning_thread_from_monitor(t_list, waitingToLockMonitor);
>> 467: // If currentThread is nullptr we would like to know if the owner
>
> Suggestion:
>
> // If currentThread is null we would like to know if the owner
Fixed.
> src/hotspot/share/services/threadService.cpp line 474:
>
>> 472: // vthread we never record this as a deadlock. Note: unless there
>> 473: // is a bug in the VM, or a thread exits without releasing monitors
>> 474: // acquired through JNI, nullptr should imply unmounted vthread owner.
>
> Suggestion:
>
> // acquired through JNI, null should imply an unmounted vthread owner.
Fixed.
> src/java.base/share/classes/java/lang/Object.java line 383:
>
>> 381: try {
>> 382: wait0(timeoutMillis);
>> 383: } catch (InterruptedException e) {
>
> I had expected to see a call to a new `wait0` method that returned a value indicating whether the wait was completed or else we had to park. Instead we had to put special logic in the native-call-wrapper code in the VM to detect returning from wait0 and changing the return address. I'm still unclear where that modified return address actually takes us.
We jump to `StubRoutines::cont_preempt_stub()`. We need to remove all the frames that were just copied to the heap from the physical stack, and then return to the calling method which will be `Continuation.run`.
> src/java.base/share/classes/java/lang/Thread.java line 654:
>
>> 652: * {@link Thread#PRIMORDIAL_TID} +1 as this class cannot be used during
>> 653: * early startup to generate the identifier for the primordial thread. The
>> 654: * counter is off-heap and shared with the VM to allow it assign thread
>
> Suggestion:
>
> * counter is off-heap and shared with the VM to allow it to assign thread
Fixed.
> src/java.base/share/classes/java/lang/Thread.java line 731:
>
>> 729:
>> 730: if (attached && VM.initLevel() < 1) {
>> 731: this.tid = 3; // primordial thread
>
> The comment before the `ThreadIdentifiers` class needs updating to account for this change.
Fixed.
> src/java.base/share/classes/java/lang/VirtualThread.java line 109:
>
>> 107: *
>> 108: * RUNNING -> BLOCKING // blocking on monitor enter
>> 109: * BLOCKING -> BLOCKED // blocked on monitor enter
>
> Should this say something similar to the parked case, about the "yield" being successful?
Since the unmount is triggered from the VM we never call yieldContinuation(), unlike with the PARKING case. In other words, there are no two cases to handle. If freezing the continuation fails, the virtual thread will already block in the monitor code pinned to the carrier, so a state of BLOCKING means freezing the continuation succeeded.
> src/java.base/share/classes/java/lang/VirtualThread.java line 110:
>
>> 108: * RUNNING -> BLOCKING // blocking on monitor enter
>> 109: * BLOCKING -> BLOCKED // blocked on monitor enter
>> 110: * BLOCKED -> UNBLOCKED // unblocked, may be scheduled to continue
>
> Does this mean it now owns the monitor, or just it is able to re-contest for monitor entry?
It means it is scheduled to run again and re-contest for the monitor.
> src/java.base/share/classes/java/lang/VirtualThread.java line 111:
>
>> 109: * BLOCKING -> BLOCKED // blocked on monitor enter
>> 110: * BLOCKED -> UNBLOCKED // unblocked, may be scheduled to continue
>> 111: * UNBLOCKED -> RUNNING // continue execution after blocked on monitor enter
>
> Presumably this one means it acquired the monitor?
Not really, it is the state we set when the virtual thread is mounted and runs again. In this case it will just run to re-contest for the monitor.
> src/java.base/share/classes/java/lang/VirtualThread.java line 631:
>
>> 629: // Object.wait
>> 630: if (s == WAITING || s == TIMED_WAITING) {
>> 631: byte nonce;
>
> Suggestion:
>
> byte seqNo;
Changed to seqNo.
> src/java.base/share/classes/java/lang/VirtualThread.java line 948:
>
>> 946: * This method does nothing if the thread has been woken by notify or interrupt.
>> 947: */
>> 948: private void waitTimeoutExpired(byte nounce) {
>
> I assume you meant `nonce` here, but please change to `seqNo`.
Changed.
> src/java.base/share/classes/java/lang/VirtualThread.java line 952:
>
>> 950: for (;;) {
>> 951: boolean unblocked = false;
>> 952: synchronized (timedWaitLock()) {
>
> Where is the overall design of the timed-wait protocol and it use of synchronization described?
When we unmount on a timed-wait call we schedule a wakeup task at the end of `afterYield`. There are two mechanisms that avoid the scheduled task to run and wake up the virtual thread on a future timed-wait call, since in this call the virtual thread could have been already notified before the scheduled task runs. The first one is to cancel the scheduled task once we return from the wait call (see `Object.wait(long timeoutMillis)`). Since the task could have been already started though, we also use `timedWaitSeqNo`, which the wake up task checks here to make sure it is not an old one. Since we synchronize on `timedWaitLock` to increment `timedWaitSeqNo` and change state to `TIMED_WAIT` before scheduling the wake up task in `afterYield`, here either a wrong `timedWaitSeqNo` or a state different than `TIMED_WAIT` means there is nothing to do. The only exception is checking for `SUSPENDED` state, in which case we just loop to retry.
> src/java.base/share/classes/java/lang/VirtualThread.java line 1397:
>
>> 1395:
>> 1396: /**
>> 1397: * Returns a lock object to coordinating timed-wait setup and timeout handling.
>
> Suggestion:
>
> * Returns a lock object for coordinating timed-wait setup and timeout handling.
Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819744051
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817192967
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817195264
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817195487
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1826154797
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1805830255
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1815700441
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1828195851
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817196602
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817197017
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817388840
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817199027
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817200025
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817200202
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811596618
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817200507
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811596855
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811600012
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1813525449
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811600739
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1814187730
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811601098
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811601168
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811601545
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817195731
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811601472
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811601619
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811601871
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1811602000
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1814187856
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817195899
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817196260
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817196374
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817200860
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817200711
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1813237094
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1826155159
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1826155815
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1815701043
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1815693906
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1813237507
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1813239314
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1813239799
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1813240352
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1815699934
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1815700133
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1817190381
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1815700312
More information about the core-libs-dev
mailing list