On Tue, 12 Nov 2024 02:59:59 GMT, Patricio Chilano Mateo
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 change
On Tue, 12 Nov 2024 02:59:59 GMT, Patricio Chilano Mateo
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 change
On Tue, 12 Nov 2024 02:59:59 GMT, Patricio Chilano Mateo
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 change
On Tue, 12 Nov 2024 02:59:59 GMT, Patricio Chilano Mateo
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 change
> 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:
>
> - Change
On Fri, 8 Nov 2024 13:43:14 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/oops/stackChunkOop.inline.hpp line 189:
>>
>>> 187: inline ObjectMonitor* stackChunkOopDesc::current_pending_monitor()
>>> const {
>>> 188: ObjectWaiter* waiter = object_waiter();
>>> 189: if (waiter != nul
On Wed, 30 Oct 2024 17:23:31 GMT, Coleen Phillimore wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Fix in JvmtiEnvBase::get_locked_objects_in_frame()
>> - Add ObjectWaiter::at_monitorenter
>
> src/hot
> 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:
>
> - Change
On Thu, 7 Nov 2024 19:15:50 GMT, Patricio Chilano Mateo
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
On Thu, 7 Nov 2024 09:45:40 GMT, Amit Kumar wrote:
> > I think we can add @requires vm.continuations to this test. It's not useful
> > to run with the alternative virtual thread implementation.
>
> Sure, that sounds ok. Thanks.
>
Added `@requires vm.continuations` to the test.
-
P
On Thu, 7 Nov 2024 18:32:14 GMT, Serguei Spitsyn wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Use JvmtiVTMSTransitionDisabler::VTMS_vthread_mount/unmount
>
> src/hotspot/share/prims/jvmtiThreadState.cp
> 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:
>
> - Change
On Thu, 7 Nov 2024 00:38:57 GMT, Patricio Chilano Mateo
wrote:
>>> the call to java_lang_Thread::set_is_in_VTMS_transition()is not needed in
>>> JvmtiUnmountBeginMark
>>>
>> Why is not needed? I guess you meant to say we should use
>> `JvmtiVTMSTransitionDisabler::set_is_in_VTMS_transition()`
On Thu, 7 Nov 2024 00:40:26 GMT, Patricio Chilano Mateo
wrote:
>>> So at some point I think we need to figure out how to make them go away ...
>>
>> Yes, the 2 extension events (`VirtualThreadMount` and
>> `VirtualThreadUnmount`) were added for testing purposes. We wanted to get
>> rid of th
On Thu, 7 Nov 2024 00:38:18 GMT, Patricio Chilano Mateo
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
On Thu, 7 Nov 2024 00:38:18 GMT, Patricio Chilano Mateo
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
On Thu, 7 Nov 2024 09:40:19 GMT, Alan Bateman wrote:
>I think we can add @requires vm.continuations to this test. It's not useful to
>run with the alternative virtual thread implementation.
Sure, that sounds ok. Thanks.
-
PR Comment: https://git.openjdk.org/jdk/pull/21565#issuecom
On Wed, 6 Nov 2024 17:38:59 GMT, Patricio Chilano Mateo
wrote:
>> Good work! I'll approve the GC related changes.
>>
>> There are some simplifications I think can be done in the ObjectMonitor
>> layer, but nothing that should go into this PR.
>>
>> Similarly, (even if some of this is preexist
On Wed, 6 Nov 2024 17:38:59 GMT, Patricio Chilano Mateo
wrote:
>> Good work! I'll approve the GC related changes.
>>
>> There are some simplifications I think can be done in the ObjectMonitor
>> layer, but nothing that should go into this PR.
>>
>> Similarly, (even if some of this is preexist
On Thu, 7 Nov 2024 00:37:17 GMT, Patricio Chilano Mateo
wrote:
>> Thank you for the comment! I'm okay with your modified suggestion in general
>> if there are no road blocks.
>>
>>> but does the specs say the event has to be posted in the context of the
>>> vthread?
>>
>> As Alan said below
On Wed, 6 Nov 2024 15:57:55 GMT, Serguei Spitsyn wrote:
> The two extension events were designed to be posted when the current thread
> identity is virtual, so this behavior > needs to be considered as a bug. My
> understanding is that it is not easy to fix.
>
If we want to post the mount/unmou
On Thu, 7 Nov 2024 00:38:40 GMT, Patricio Chilano Mateo
wrote:
>>> So, it feels like it should not be a problem. I'm thinking if adding an
>>> assert at the VTMS transition end would help.
>>>
>> The problem here is that for monitorenter the top frame will not be a native
>> method, so the bai
On Thu, 7 Nov 2024 00:37:53 GMT, Patricio Chilano Mateo
wrote:
>> Great, I applied the suggested simplification. I had to update test
>> `VThreadEventTest.java` to check the stack during the mount/unmount events
>> to only count the real cases. This is because now we are getting a variable
>>
On Wed, 6 Nov 2024 16:31:24 GMT, Serguei Spitsyn wrote:
>> Regarding the pop_frame/early_ret/async_exception conditions, not checking
>> for them after we started the transition would be an issue.
>> For pop_frame/early_ret checks, the problem is that if any of them are
>> installed in `JvmtiUn
> 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:
>
> - Change
On Sat, 26 Oct 2024 05:39:32 GMT, Alan Bateman wrote:
>> test/jdk/java/lang/reflect/callerCache/ReflectionCallerCacheTest.java line
>> 30:
>>
>>> 28: * by reflection API
>>> 29: * @library /test/lib/
>>> 30: * @requires vm.compMode != "Xcomp"
>>
>> If there is a problem with this te
On Tue, 29 Oct 2024 02:09:24 GMT, Dean Long 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 sp
On Mon, 28 Oct 2024 00:29:25 GMT, David Holmes wrote:
>> I was wondering what this was too but the _previous_owner_tid is the os
>> thread id, not the Java thread id.
>>
>>
>> $ grep -r JFR_THREAD_ID
>> jfr/support/jfrThreadId.hpp:#define JFR_THREAD_ID(thread)
>> (JfrThreadLocal::external_thr
On Mon, 4 Nov 2024 07:59:22 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/jdk/internal/vm/Continuation.java line 62:
>>
>>> 60: NATIVE(2, "Native frame or on stack"),
>>> 61: MONITOR(3, "Monitor held"),
>>> 62: CRITICAL_SECTION(4, "In critical section");
>>
>>
On Thu, 17 Oct 2024 14:28:30 GMT, Patricio Chilano Mateo
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 hav
On Thu, 24 Oct 2024 22:13:27 GMT, David Holmes wrote:
>> We don't unmount the virtual thread here, we just temporarily change the
>> thread identity. You could think of this method as
>> switchIdentityToCarrierThread if that helps.
>
> Sorry to belabour this but why are we temporarily changing
On Wed, 6 Nov 2024 06:36:58 GMT, Axel Boldt-Christmas
wrote:
> A small note on `_cont_fastpath`, as it is now also used for synchronised
> native method calls (native wrapper) maybe the comment should be updated to
> reflect this.
>
> ```
> // the sp of the oldest known interpreted/call_stub
On Thu, 24 Oct 2024 02:55:18 GMT, David Holmes wrote:
>>> Also JavaThread::_lock_id in the VM means "the java.lang.Thread thread-id
>>> to use for locking" - correct?
>>>
>> Yes.
>
> I guess I don't understand where this piece code fits in the overall
> transition of the virtual thread to being
On Mon, 28 Oct 2024 09:19:48 GMT, Alan Bateman wrote:
>> Thanks for the explanation but that needs to be documented somewhere.
>
> The comment in afterYield has been expanded in the loom repo, we may be able
> to bring that update in.
Brought the comment from the loom repo.
-
PR R
On Wed, 23 Oct 2024 05:18:10 GMT, David Holmes wrote:
>> Thread identity switches to the carrier so Thread.currentThread() is the
>> carrier thread and JavaThread._lock_id is the thread identifier of the
>> carrier. setCurrentLockId changes JavaThread._lock_id back to the virtual
>> thread's
On Thu, 24 Oct 2024 21:08:47 GMT, Patricio Chilano Mateo
wrote:
>> I guess I don't understand where this piece code fits in the overall
>> transition of the virtual thread to being parked. I would have expected the
>> LockStack to already have been moved by the time we switch identities to the
On Wed, 23 Oct 2024 20:34:48 GMT, Patricio Chilano Mateo
wrote:
>> If the virtual thread is un-mounting from the carrier, why do we need to set
>> the "lock id" back to the virtual thread's id? Sorry I'm finding this quite
>> confusing.
>>
>> Also `JavaThread::_lock_id` in the VM means "the j
On Mon, 21 Oct 2024 08:01:09 GMT, Andrey Turbanov 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 b
On Thu, 31 Oct 2024 20:28:06 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/sun/security/ssl/X509TrustManagerImpl.java line
>> 57:
>>
>>> 55: static {
>>> 56: try {
>>> 57:
>>> MethodHandles.lookup().ensureInitialized(AnchorCertificates.class);
>>
>> Why is th
On Thu, 31 Oct 2024 01:32:19 GMT, Dean Long 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 sp
On Wed, 23 Oct 2024 20:36:23 GMT, Patricio Chilano Mateo
wrote:
>> Sorry, I should add context on why this is needed. The problem is that
>> inside this temporal transition we could try to acquire some monitor. If the
>> monitor is not inflated we will try to use the LockStack, but the LockSta
On Wed, 23 Oct 2024 11:32:54 GMT, Alan Bateman wrote:
>> Suggestion: `timedWaitCounter` ?
>
> We could rename it to timedWaitSeqNo if needed.
Ok, renamed to timedWaitSeqNo.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1813240667
On Mon, 28 Oct 2024 00:38:39 GMT, David Holmes wrote:
>> I thought locking_thread there may not be the current thread for enter_for()
>> in deopt. It's the thread that should hold the lock but not the current
>> thread. But it might be different now.
>
> The thread passed in need not be the c
On Thu, 24 Oct 2024 08:26:12 GMT, Alan Bateman wrote:
>> So really UNBLOCKED is UNBLOCKING and mirrors BLOCKING , so we have:
>>
>> RUNNING -> BLOCKING -> BLOCKED
>> BLOCKED -> UNBLOCKING -> RUNNABLE
>>
>> I'm just trying to get a better sense of what we can infer if we see these
>> "transitio
On Wed, 23 Oct 2024 06:11:26 GMT, David Holmes wrote:
>> Sequence number, nouce, anything will work here as it's just to deal with
>> the scenario where the timeout task for a previous wait may run concurrently
>> with a subsequent wait.
>
> Suggestion: `timedWaitCounter` ?
We could rename it
On Tue, 22 Oct 2024 12:31:24 GMT, Alan Bateman wrote:
>> Okay but
>> 1. We have the current virtual thread
>> 2. We have the current carrier for that virtual thread (which is iotself a
>> java.alng.Thread object
>> 3. We have Thread.setCurrentLockId which ... ? which thread does it update?
On Thu, 24 Oct 2024 02:47:14 GMT, David Holmes wrote:
>> 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.
>
> So really UNBLOCKED is UNBLOCKING and mirrors BLOCKING , so we have:
>
> RUNNING ->
On Tue, 22 Oct 2024 11:52:46 GMT, Alan Bateman wrote:
>> src/java.base/share/classes/java/lang/VirtualThread.java line 115:
>>
>>> 113: * RUNNING -> WAITING// transitional state during wait
>>> on monitor
>>> 114: * WAITING -> WAITED // waiting on monitor
>>> 115:
On Mon, 28 Oct 2024 11:59:57 GMT, Coleen Phillimore wrote:
>> The thread passed in need not be the current thread, and IIUC is the thread
>> that should become the owner of the newly inflated monitor (either current
>> thread or a suspended thread). The actual inflation is always done by the
>
On Fri, 25 Oct 2024 22:29:56 GMT, Coleen Phillimore wrote:
>>> If it's always the current thread, then it should be called 'current' imo.
>>>
>> The inflating thread is always the current one but it's not always equal to
>> `inflating_thread`.
>
> I thought locking_thread there may not be the cu
On Fri, 25 Oct 2024 18:46:52 GMT, Patricio Chilano Mateo
wrote:
>> 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 cas
On Fri, 25 Oct 2024 21:28:22 GMT, Patricio Chilano Mateo
wrote:
>> If it's always the current thread, then it should be called 'current' imo.
>
> I see that in lightweightSynchronizer.cpp we already use the name
> `locking_thread` (although
> `LightweightSynchronizer::inflate_into_object_heade
On Wed, 6 Nov 2024 00:01:21 GMT, Patricio Chilano Mateo
wrote:
>>> Is this posted after the VirtualThreadMount extension event posted?
>>>
>> It's posted before. We post the mount event at the end of thaw only if we
>> are able to acquire the monitor:
>> https://github.com/openjdk/jdk/blob/12
On Fri, 25 Oct 2024 21:29:05 GMT, Patricio Chilano Mateo
wrote:
>> I see that in lightweightSynchronizer.cpp we already use the name
>> `locking_thread` (although
>> `LightweightSynchronizer::inflate_into_object_header` still uses
>> `inflating_thread`). So how about using `locking_thread` in
On Thu, 24 Oct 2024 08:08:56 GMT, Stefan Karlsson 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 b
On Fri, 25 Oct 2024 12:00:43 GMT, Coleen Phillimore wrote:
>> src/hotspot/share/runtime/synchronizer.cpp line 1440:
>>
>>> 1438: }
>>> 1439:
>>> 1440: ObjectMonitor* ObjectSynchronizer::inflate_impl(JavaThread*
>>> inflating_thread, oop object, const InflateCause cause) {
>>
>> `inflating_thr
On Thu, 17 Oct 2024 14:28:30 GMT, Patricio Chilano Mateo
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 hav
On Thu, 31 Oct 2024 02:26:42 GMT, David Holmes wrote:
>> src/hotspot/share/runtime/objectMonitor.inline.hpp line 207:
>>
>>> 205: }
>>> 206:
>>> 207: inline bool ObjectMonitor::has_successor() {
>>
>> Why are _succ accesses atomic here when previously they were not?
>
> General convention is t
On Fri, 25 Oct 2024 11:59:03 GMT, Coleen Phillimore wrote:
>> src/hotspot/share/runtime/objectMonitor.hpp line 174:
>>
>>> 172:
>>> 173: int64_t volatile _owner; // Either tid of owner, NO_OWNER,
>>> ANONYMOUS_OWNER or DEFLATER_MARKER.
>>> 174: volatile uint64_t _previous_owner_tid; // t
On Thu, 31 Oct 2024 16:34:41 GMT, Patricio Chilano Mateo
wrote:
>> General convention is that racily accessed variables should be accessed via
>> Atomic::load/store to make it clear(er) they are racy accesses. But I agree
>> it seems odd when direct accesses to `_succ` in the main cpp file are
On Wed, 23 Oct 2024 20:42:44 GMT, Patricio Chilano Mateo
wrote:
>> 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);
On Mon, 28 Oct 2024 13:08:37 GMT, Richard Reingruber 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 hav
On Mon, 28 Oct 2024 13:12:22 GMT, Richard Reingruber wrote:
>> src/hotspot/share/runtime/objectMonitor.hpp line 202:
>>
>>> 200:
>>> 201: // Used in LM_LEGACY mode to store BasicLock* in case of inflation
>>> by contending thread.
>>> 202: BasicLock* volatile _stack_locker;
>>
>> IIUC the
On Wed, 23 Oct 2024 09:53:53 GMT, Richard Reingruber 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 hav
On Mon, 28 Oct 2024 00:35:11 GMT, David Holmes wrote:
>> 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 a
On Tue, 22 Oct 2024 19:02:50 GMT, Patricio Chilano Mateo
wrote:
>> Just to say that we hope to eventually remove these "temporary transitions".
>> This PR brings in a change that we've had in the loom repo to not need this
>> when calling out to the scheduler. The only significant remaining us
On Wed, 23 Oct 2024 05:33:55 GMT, Axel Boldt-Christmas
wrote:
>> Ok, I'll change copy_lockstack to both load and clear the oops in the same
>> method. Now, when we call do_barriers on recurse_thaw we don't clear the
>> oops, we just load and store the loaded value again. Is it the case that we
On Wed, 6 Nov 2024 09:24:03 GMT, Alan Bateman wrote:
> So at some point I think we need to figure out how to make them go away ...
Yes, the 2 extension events (`VirtualThreadMount` and `VirtualThreadUnmount`)
were added for testing purposes. We wanted to get rid of them at some point but
the
On Wed, 23 Oct 2024 09:53:44 GMT, Alan Bateman wrote:
>> The problem is that within that window we don't have access to the virtual
>> thread's tid. The current thread has already been changed and we haven't yet
>> set the lock id back. Since this will be a rare corner case maybe we can
>> jus
On Tue, 22 Oct 2024 19:04:16 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 2234:
>>
>>> 2232: retry_fast_path = true;
>>> 2233: } else {
>>> 2234: relativize_chunk_concurrently(chunk);
>>
>> Is the `relativize_chunk_concurrently` solu
On Mon, 21 Oct 2024 15:41:45 GMT, Axel Boldt-Christmas
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
On Wed, 23 Oct 2024 00:56:34 GMT, Coleen Phillimore 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
On Tue, 22 Oct 2024 11:51:47 GMT, Alan Bateman wrote:
>> src/hotspot/share/runtime/javaThread.cpp line 1545:
>>
>>> 1543: if (is_vthread_mounted()) {
>>> 1544: // _lock_id is the thread ID of the mounted virtual thread
>>> 1545: st->print_cr(" Carrying virtual thread #" INT64_F
On Wed, 30 Oct 2024 19:02:05 GMT, Coleen Phillimore wrote:
>> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1411:
>>
>>> 1409: // zero out fields (but not the stack)
>>> 1410: const size_t hs = oopDesc::header_size();
>>> 1411: oopDesc::set_klass_gap(mem, 0);
>>
>> Why,
On Wed, 23 Oct 2024 09:58:44 GMT, Alan Bateman wrote:
>> 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.
>>> 166:
On Tue, 5 Nov 2024 23:53:04 GMT, Patricio Chilano Mateo
wrote:
>> Yes, I see your idea to get rid of the pending unmount event code. Before
>> commenting on that, note that we still need to check if the freeze failed to
>> undo the transition, which would call for this RAII object that we curr
On Wed, 30 Oct 2024 23:14:53 GMT, Patricio Chilano Mateo
wrote:
>> This might confuse the change for JEP 450 since with CompactObjectHeaders
>> there's no klass_gap, so depending on which change goes first, there will be
>> conditional code here. Good question though, it looks like we only eve
On Fri, 25 Oct 2024 13:12:11 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1275:
>>
>>> 1273:
>>> 1274: if (caller.is_interpreted_frame()) {
>>> 1275: _total_align_size += frame::align_wiggle;
>>
>> Please put a comment here about frame
On Thu, 31 Oct 2024 21:11:39 GMT, Fredrik Bredberg
wrote:
>> There was one value that meant to be for the regular freeze from java. But
>> it was not used so I removed it.
>
> Fair enough, but I would prefer if you start at zero. Just so people like me
> don't start scratching their head tryin
On Wed, 30 Oct 2024 20:10:03 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/runtime/continuation.cpp line 88:
>>
>>> 86: if (_target->has_async_exception_condition()) {
>>> 87: _failed = true;
>>> 88: }
>>
>> Q: I wonder why the failed conditions are not checked be
On Thu, 31 Oct 2024 20:05:18 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/runtime/continuation.hpp line 66:
>>
>>> 64:
>>> 65: enum preempt_kind {
>>> 66: freeze_on_monitorenter = 1,
>>
>> Is there a reason why the first enumerator doesn't start at zero?
>
> There was one val
On Tue, 5 Nov 2024 23:50:29 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/runtime/continuation.cpp line 134:
>>
>>> 132: return true;
>>> 133: }
>>> 134: #endif // INCLUDE_JVMTI
>>
>> Could you, please, consider the simplification below?
>>
>>
>> #if INCLUDE_JVMTI
>> // return tr
On Mon, 4 Nov 2024 09:24:13 GMT, Stefan Karlsson wrote:
>> If I recall correctly this was a bug where one of the stackChunk fields was
>> allocated in that gap, but since we didn't zeroed it out it would start with
>> some invalid value. I guess the reason why we are not hitting this today is
On Thu, 17 Oct 2024 14:28:30 GMT, Patricio Chilano Mateo
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 hav
On Tue, 29 Oct 2024 02:56:30 GMT, Serguei Spitsyn 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 b
On Mon, 28 Oct 2024 10:37:21 GMT, Yudi Zheng 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 s
On Tue, 5 Nov 2024 07:51:05 GMT, Erik Gahlin 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 s
On Tue, 5 Nov 2024 07:48:40 GMT, Erik Gahlin 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 s
On Thu, 17 Oct 2024 14:28:30 GMT, Patricio Chilano Mateo
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 hav
On Mon, 4 Nov 2024 05:52:16 GMT, Alan Bateman wrote:
>> src/hotspot/share/classfile/javaClasses.cpp line 2107:
>>
>>> 2105:
>>> 2106: jlong java_lang_VirtualThread::waitTimeout(oop vthread) {
>>> 2107: return vthread->long_field(_timeout_offset);
>>
>> Not sure what motivated the name change
On Thu, 17 Oct 2024 14:28:30 GMT, Patricio Chilano Mateo
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 hav
On Tue, 5 Nov 2024 08:19:34 GMT, Alan Bateman wrote:
>> src/hotspot/share/jfr/metadata/metadata.xml line 160:
>>
>>> 158:
>>> 159:
>>> 160:
>>
>> Previously, the event was in the "Java Application" category. I think that
>> was a better fit because it meant it was visualized in the sa
On Mon, 4 Nov 2024 02:12:40 GMT, David Holmes 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
On Mon, 28 Oct 2024 22:02:02 GMT, Patricio Chilano Mateo
wrote:
>> That said such a scenario is not about concurrently pushing the same thread
>> to the list from different threads. So I'm still somewhat confused about the
>> concurrency control here. Specifically I can't see how the cmpxchg o
On Mon, 28 Oct 2024 00:55:34 GMT, David Holmes wrote:
>> H ... I guess we either slow down the monitor code by having the thread
>> search for and remove itself, or we allow for this and handle it correctly
>> ... okay.
>
> That said such a scenario is not about concurrently pushing the sa
On Mon, 28 Oct 2024 00:43:47 GMT, David Holmes wrote:
>>> I'm struggling to understand how a thread can already be on this list?
>>>
>> With the removal of the _Responsible thread, it's less likely but it could
>> still happen. One case is when the virtual thread acquires the monitor after
>> a
On Fri, 25 Oct 2024 18:39:54 GMT, Patricio Chilano Mateo
wrote:
>>> The "waiting list" here is just a list of virtual threads that need
>>> unparking by the Unblocker thread - right?
>>>
>> Yes.
>
>> Some comments here about the operation would be useful.
>>
> Added a comment.
> I'm struggling
On Fri, 25 Oct 2024 18:40:51 GMT, Patricio Chilano Mateo
wrote:
>>> Some comments here about the operation would be useful.
>>>
>> Added a comment.
>
>> I'm struggling to understand how a thread can already be on this list?
>>
> With the removal of the _Responsible thread, it's less likely but i
On Fri, 25 Oct 2024 18:39:23 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/classfile/javaClasses.cpp line 2082:
>>
>>> 2080: }
>>> 2081:
>>> 2082: bool java_lang_VirtualThread::set_onWaitingList(oop vthread,
>>> OopHandle& list_head) {
>>
>> Some comments here about the operation w
On Mon, 28 Oct 2024 07:55:02 GMT, Richard Reingruber 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 hav
1 - 100 of 488 matches
Mail list logo