On Thu, 31 Oct 2024 03:52:31 GMT, Dean Long wrote:
> For some reason github thinks VirtualThreadPinnedEvent.java was renamed to
> libSynchronizedNative.c and libTracePinnedThreads.c was renamed to
> LockingMode.java. Is there a way to fix that?
I don't know which view this is but just to say t
On Mon, 28 Oct 2024 23:46:09 GMT, Dean Long wrote:
> > regardless of when you freeze, while doing the freezing the monitor could
> > have been released already. So trying to acquire the monitor after freezing
> > can always succeed, which means we don't want to unmount but continue
> > executi
On Tue, 5 Nov 2024 01:47:29 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, 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, 28 Oct 2024 17:31:45 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp line 188:
>>
>>> 186: // Avoid using a leave instruction when this frame may
>>> 187: // have been frozen, since the current value of rfp
>>> 188: // restored from the stub w
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 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
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 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: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 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 Tue, 22 Oct 2024 02:14:23 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/cpu/x86/assembler_x86.cpp line 2866:
>>
>>> 2864: emit_int32(0);
>>> 2865: }
>>> 2866: }
>>
>> Is it possible to make this more general and explicit instead of a sequence
>> of bytes?
>>
>> Something along t
On Mon, 28 Oct 2024 01:13:05 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 20:49:45 GMT, Patricio Chilano Mateo
wrote:
>> 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 che
On Mon, 28 Oct 2024 21:13:33 GMT, Patricio Chilano Mateo
wrote:
>> If preemption was cancelled, we skip over the cleanup. The native frames
>> haven't been unwound yet. So when we call thaw, does it cleanup the native
>> frames first, or does it copy the frames back on top of the existing fr
On Mon, 28 Oct 2024 23:09:58 GMT, Dean Long wrote:
>> 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 appearin
On Tue, 29 Oct 2024 08:29:55 GMT, David Holmes wrote:
>> It's conceivable that in the future we might have more native methods we
>> want to preempt. Instead of enumerating them all, we could set a flag on
>> the method.
>>
>> I was assuming that David was suggesting we have the Java caller d
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, 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 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 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
> 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 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
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 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 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 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 Wed, 6 Nov 2024 06:53:34 GMT, David Holmes wrote:
>> SendaoYan has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> change os-arch to generic-all
>
> test/jdk/ProblemList.txt line 779:
>
>> 777: java/lang/Thread/jni/AttachCurrentThread/At
> Hi all,
> To make less CI noisy, before the root cause of
> [JDK-8343244](https://bugs.openjdk.org/browse/JDK-8343244) has fixed, should
> we problemlist the failure tests, include AttachTest.java and
> TestZPageAllocationEvent.java
SendaoYan has updated the pull request incrementally with on
On Wed, 6 Nov 2024 08:11:07 GMT, SendaoYan wrote:
>> Hi all,
>> To make less CI noisy, before the root cause of
>> [JDK-8343244](https://bugs.openjdk.org/browse/JDK-8343244) has fixed, should
>> we problemlist the failure tests, include AttachTest.java and
>> TestZPageAllocationEvent.java
>
>
On Wed, 6 Nov 2024 06:55:41 GMT, David Holmes wrote:
>> SendaoYan has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 1. change os-arch to linux-all; 2. delete
>> jdk/jfr/event/gc/detailed/TestZPageAllocationEvent.java from Problemlist
>
>
> Hi all,
> To make less CI noisy, before the root cause of
> [JDK-8343244](https://bugs.openjdk.org/browse/JDK-8343244) has fixed, should
> we problemlist the failure tests, include AttachTest.java and
> TestZPageAllocationEvent.java
SendaoYan has updated the pull request incrementally with on
> This is the main body of the JEP 450: Compact Object Headers (Experimental).
>
> It is also a follow-up to #20640, which now also includes (and supersedes)
> #20603 and #20605, plus the Tiny Class-Pointers parts that have been
> previously missing.
>
> Main changes:
> - Introduction of the (
On Tue, 5 Nov 2024 16:43:35 GMT, Roman Kennke wrote:
>> @egahlin / @mgronlun could you please review the JFR parts of this PR? One
>> change is for getting the right prototype header, the other is for avoiding
>> an endless loop/assert in a corner case.
>
>> @rkennke can you include this small
On Wed, 6 Nov 2024 09:12:02 GMT, Thomas Stuefe wrote:
> > I think you may be throwing the baby out with the bath water when it comes
> > to `__stdcall`. It may be that 32-bit requires `__stdcall` but I don't see
> > anything that states `__stdcall` is ONLY for 32-bit!
>
> stdcall and cdecl are
On Wed, 6 Nov 2024 04:40:24 GMT, Julian Waters wrote:
> I think you may be throwing the baby out with the bath water when it comes to
> `__stdcall`. It may be that 32-bit requires `__stdcall` but I don't see
> anything that states `__stdcall` is ONLY for 32-bit!
stdcall and cdecl are 32-bit Wi
On Tue, 5 Nov 2024 08:58:00 GMT, Kim Barrett wrote:
>> Magnus Ihse Bursie has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> fix: jvm_md.h was included, but not jvm.h...
>
> src/hotspot/os/windows/os_windows.cpp line 5863:
>
>> 5861: r
> This is the implementation of [JEP 479: _Remove the Windows 32-bit x86
> Port_](https://openjdk.org/jeps/479).
>
> This is the summary of JEP 479:
>> Remove the source code and build support for the Windows 32-bit x86 port.
>> This port was [deprecated for removal in JDK
>> 21](https://openjd
On Tue, 5 Nov 2024 16:28:04 GMT, Kim Barrett wrote:
>> Magnus Ihse Bursie has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> fix: jvm_md.h was included, but not jvm.h...
>
> src/hotspot/os/windows/os_windows.cpp line 5820:
>
>> 5818: }
>>
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 Tue, 5 Nov 2024 21:00:07 GMT, Alex Menkov wrote:
>> The fix adds correct handling of "could not open pipe" errors in attach
>> listener instead of assert which causes target VM crash.
>>
>> testing: tier1,tier2,hs-tier5-svc
>
> Alex Menkov has updated the pull request incrementally with one
> Hi all,
> To make less CI noisy, before the root cause of
> [JDK-8343244](https://bugs.openjdk.org/browse/JDK-8343244) has fixed, should
> we problemlist the failure tests, include AttachTest.java and
> TestZPageAllocationEvent.java
SendaoYan has updated the pull request incrementally with on
On Wed, 6 Nov 2024 08:19:42 GMT, Alan Bateman wrote:
>> SendaoYan has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 1. change os-arch to linux-all; 2. delete
>> jdk/jfr/event/gc/detailed/TestZPageAllocationEvent.java from Problemlist
>
>
On Thu, 7 Nov 2024 01:23:32 GMT, Leonid Mesnik wrote:
> Please update bug/PR summary to reflect actual changes.
Thanks for the remind. The bug and PR summary has been updated.
-
PR Comment: https://git.openjdk.org/jdk/pull/21917#issuecomment-2461149994
On Wed, 6 Nov 2024 08:39:08 GMT, SendaoYan wrote:
>> Hi all,
>> To make less CI noisy, before the root cause of
>> [JDK-8343244](https://bugs.openjdk.org/browse/JDK-8343244) has fixed, should
>> we problemlist the failure tests, include AttachTest.java and
>> TestZPageAllocationEvent.java
>
>
On Wed, 6 Nov 2024 08:39:08 GMT, SendaoYan wrote:
>> Hi all,
>> To make less CI noisy, before the root cause of
>> [JDK-8343244](https://bugs.openjdk.org/browse/JDK-8343244) has fixed, should
>> we problemlist the failure tests, include AttachTest.java and
>> TestZPageAllocationEvent.java
>
>
On Wed, 30 Oct 2024 17:20:49 GMT, Larry Cable wrote:
>> the implementation I originally provided does not in fact solve the issue!
>>
>> the attach protocol initiation "handshake" requires that the "attacher" (the
>> caller of this code) and the "attachee"(the target JVM to be "attached" to)
>
On Wed, 30 Oct 2024 17:20:49 GMT, Larry Cable wrote:
>> the implementation I originally provided does not in fact solve the issue!
>>
>> the attach protocol initiation "handshake" requires that the "attacher" (the
>> caller of this code) and the "attachee"(the target JVM to be "attached" to)
>
On Sat, 26 Oct 2024 02:15:29 GMT, Dean Long wrote:
> > On failure to acquire a monitor inside `ObjectMonitor::enter` a virtual
> > thread will call freeze to copy all Java frames to the heap. We will add
> > the virtual thread to the ObjectMonitor's queue and return back to Java.
> > Instead o
On Mon, 28 Oct 2024 18:58:29 GMT, Patricio Chilano Mateo
wrote:
> regardless of when you freeze, while doing the freezing the monitor could
> have been released already. So trying to acquire the monitor after freezing
> can always succeed, which means we don't want to unmount but continue
> e
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 Wed, 23 Oct 2024 06:15:27 GMT, David Holmes wrote:
> Why do we need to cache it? Is it the implicit barriers related to accessing
> the threadObj oop each time?
We cache threadObj.thread_id in JavaThread::_lock_id so that the fast path
c2_MacroAssembler code has one less load and code to fi
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, 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 Sat, 26 Oct 2024 01:51:12 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 Fri, 25 Oct 2024 13:11:38 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 2032:
>>
>>> 2030: // Force freeze slow path in case we try to preempt. We will pin
>>> the
>>> 2031: // vthread to the carrier (see
>>> FreezeBase::recurse_freeze
On Tue, 29 Oct 2024 10:06:01 GMT, Fredrik Bredberg
wrote:
>> Right. We want to take the slow path to find the compiled native wrapper
>> frame and fail to freeze. Otherwise the fast path won't find it since we
>> don't walk the stack.
>
> It would be nice if Coleen's question and your answer c
On Mon, 28 Oct 2024 16:39:14 GMT, Coleen Phillimore wrote:
>> src/hotspot/cpu/aarch64/stackChunkFrameStream_aarch64.inline.hpp line 119:
>>
>>> 117: return mask.num_oops()
>>> 118: + 1 // for the mirror oop
>>> 119: + (f.interpreter_frame_method()->is_native() ? 1 : 0) // temp
On Mon, 28 Oct 2024 20:10:16 GMT, Dean Long wrote:
>> It's the offset of the mirror passed to static native calls. It pre-existed
>> saving the mirror in all frames to keep the Method alive, and is duplicated.
>> I think this could be cleaned up someday, which would remove this special
>> ca
On Mon, 28 Oct 2024 21:01:47 GMT, Patricio Chilano Mateo
wrote:
>> I tried to track down how interpreter_frame_num_oops() is used, and as far
>> as I can tell, it is only used to compare against the bitmap in debug/verify
>> code. So if this slot was added here, shouldn't there be a correspon
On Mon, 28 Oct 2024 22:52:40 GMT, Coleen Phillimore wrote:
>> When creating the bitmap, processing oops in an interpreter frame is done
>> with `frame::oops_interpreted_do()` which already counts this extra oop for
>> native methods.
>
> What are we counting now with MaskFillerForNativeFrame th
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, 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, 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, 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, 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, 17 Oct 2024 14:28:30 GMT, Patricio Chilano Mateo
wrote:
> * 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
On Tue, 22 Oct 2024 15:23:50 GMT, Andrew Haley wrote:
> This last sentence has interesting consequences for user-defined schedulers.
> Would it make sense to throw an exception if a carrier thread is holding a
> monitor while mounting a virtual thread? Doing that would also have the
> advantag
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 Tue, 5 Nov 2024 21:00:07 GMT, Alex Menkov wrote:
>> The fix adds correct handling of "could not open pipe" errors in attach
>> listener instead of assert which causes target VM crash.
>>
>> testing: tier1,tier2,hs-tier5-svc
>
> Alex Menkov has updated the pull request incrementally with one
On Mon, 28 Oct 2024 23:21:14 GMT, Dean Long wrote:
>> What are we counting now with MaskFillerForNativeFrame that we weren't
>> counting before this change? in MaskFillerForNative::set_one.
>
> So it sounds like the adjustment at line 119 is a bug fix, but what I don't
> understand is why we w
On Mon, 28 Oct 2024 23:59:55 GMT, Patricio Chilano Mateo
wrote:
>> So it sounds like the adjustment at line 119 is a bug fix, but what I don't
>> understand is why we weren't seeing problems before. Something in this PR
>> exposed the need for this change.
>
>> What are we counting now with M
On Sat, 2 Nov 2024 02:41:44 GMT, Fei Yang wrote:
>> Changed.
>
> Note that `frame::sender_sp_offset` is 0 instead of 2 on linux-riscv64, which
> is different from aarch64 or x86-64. So I think we should revert this change:
> https://github.com/openjdk/jdk/pull/21565/commits/12213a70c1cf0639555f
On Thu, 31 Oct 2024 20:02:31 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/cpu/riscv/continuationFreezeThaw_riscv.inline.hpp line 273:
>>
>>> 271: ? frame_sp + fsize - frame::sender_sp_offset
>>> 272: // we need to re-read fp because it may be an oop and we might
>>> have f
On Sat, 2 Nov 2024 02:41:44 GMT, Fei Yang wrote:
>> Changed.
>
> Note that `frame::sender_sp_offset` is 0 instead of 2 on linux-riscv64, which
> is different from aarch64 or x86-64. So I think we should revert this change:
> https://github.com/openjdk/jdk/pull/21565/commits/12213a70c1cf0639555f
On Mon, 4 Nov 2024 18:23:23 GMT, Patricio Chilano Mateo
wrote:
>> Sorry, I also thought it matched the aarch64 one without checking.
>> @RealFYang should I change it for `hf.sp() + frame::link_offset` or just
>> leave it as it was?
>
>> Also, does this mean that the changes from 2 to frame::se
On Mon, 4 Nov 2024 18:22:42 GMT, Patricio Chilano Mateo
wrote:
>> Note that `frame::sender_sp_offset` is 0 instead of 2 on linux-riscv64,
>> which is different from aarch64 or x86-64. So I think we should revert this
>> change:
>> https://github.com/openjdk/jdk/pull/21565/commits/12213a70c1cf
On Tue, 5 Nov 2024 00:23:37 GMT, Fei Yang wrote:
>>> As the same code on aarch64 and x86-64 uses `frame::sender_sp_offset` I
>>> suggested to change the literal 2 into `frame::sender_sp_offset` in order
>>> to increase the readability, but I forgot that `frame::sender_sp_offset` is
>>> 0 on ri
On Tue, 5 Nov 2024 00:18:17 GMT, Fei Yang wrote:
>>> Also, does this mean that the changes from 2 to frame::sender_sp_offset in
>>> all of the lines (267, 271 and 273) should be reverted?
>>>
>> I think the previous lines are okay because we are constructing the fp,
>> whereas in here we want t
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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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, 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 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 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 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 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
1 - 100 of 185 matches
Mail list logo
