Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v5]

2024-11-12 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v5]

2024-11-12 Thread Axel Boldt-Christmas
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v5]

2024-11-11 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v5]

2024-11-11 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v5]

2024-11-11 Thread Patricio Chilano Mateo
> 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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v4]

2024-11-08 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v4]

2024-11-08 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v4]

2024-11-08 Thread Patricio Chilano Mateo
> 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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v3]

2024-11-07 Thread Serguei Spitsyn
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v3]

2024-11-07 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-07 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v3]

2024-11-07 Thread Patricio Chilano Mateo
> 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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-07 Thread Serguei Spitsyn
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()`

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-07 Thread Serguei Spitsyn
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-07 Thread Serguei Spitsyn
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-07 Thread Serguei Spitsyn
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-07 Thread Amit Kumar
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-07 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-07 Thread Amit Kumar
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-06 Thread Patricio Chilano Mateo
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 >>

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v2]

2024-11-06 Thread Patricio Chilano Mateo
> 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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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"); >> >>

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Andrey Turbanov
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Coleen Phillimore
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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?

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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 ->

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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:

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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 >

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Coleen Phillimore
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Stefan Karlsson
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Coleen Phillimore
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);

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Richard Reingruber
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Serguei Spitsyn
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Axel Boldt-Christmas
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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,

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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:

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Serguei Spitsyn
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Stefan Karlsson
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Coleen Phillimore
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Serguei Spitsyn
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Fredrik Bredberg
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Serguei Spitsyn
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Yudi Zheng
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Erik Gahlin
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Alan Bateman
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread David Holmes
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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

Re: RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning

2024-11-06 Thread Patricio Chilano Mateo
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   2   3   4   5   >