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
> 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, 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 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 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 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 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 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 17:32:45 GMT, Patricio Chilano Mateo
wrote:
>> src/java.base/share/classes/java/lang/VirtualThread.java line 111:
>>
>>> 109: * BLOCKING -> BLOCKED// blocked on monitor enter
>>> 110: * BLOCKED -> UNBLOCKED // unblocked, may be scheduled to
>>> con
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 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 Tue, 22 Oct 2024 15:50:15 GMT, Andrew Haley wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with six
>> additional commits since the last revision:
>>
>> - Fix comments in objectMonitor.hpp
>> - Move frame::saved_thread_address() to platform dependent files
>>
On Wed, 23 Oct 2024 05:56:48 GMT, Axel Boldt-Christmas
wrote:
>> Also, is it better to have this without assignment. Which is a nit.
>> Address dst(rthread, JavaThread::held_monitor_count_offset());
>
> The `=` in a variable definition is always construction, never assignment.
>
> That said,
On Tue, 22 Oct 2024 15:56:21 GMT, Andrew Haley wrote:
>> Note also that `inc_held_monitor_count` clobbers `rscratch2`. That might be
>> worth a comment at the call site.
>> I guess `inc_held_monitor_count` is so hot that we can't push and pop
>> scratch registers, in which case it'd clobber no
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 Tue, 22 Oct 2024 07:03:48 GMT, David Holmes wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with six
>> additional commits since the last revision:
>>
>> - Fix comments in objectMonitor.hpp
>> - Move frame::saved_thread_address() to platform dependent files
>>
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, 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 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, 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 Wed, 23 Oct 2024 00:35:19 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/runtime/objectMonitor.hpp line 292:
>>
>>> 290:
>>> 291: static int64_t owner_for(JavaThread* thread);
>>> 292: static int64_t owner_for_oop(oop vthread);
>>
>> Some comments describing this API would be
On Wed, 23 Oct 2024 00:08:54 GMT, Coleen Phillimore wrote:
>> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5341:
>>
>>> 5339:
>>> 5340: void MacroAssembler::inc_held_monitor_count() {
>>> 5341: Address dst = Address(rthread,
>>> JavaThread::held_monitor_count_offset());
>>
>> Sug
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 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 Wed, 23 Oct 2024 00:37:25 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/share/runtime/objectMonitor.hpp line 315:
>>
>>> 313: void set_succesor(oop vthread);
>>> 314: void clear_succesor();
>>> 315: bool has_succesor();
>>
>> Sorry but `successor` has two `s` before
On Tue, 22 Oct 2024 15:49:32 GMT, Andrew Haley wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with six
>> additional commits since the last revision:
>>
>> - Fix comments in objectMonitor.hpp
>> - Move frame::saved_thread_address() to platform dependent files
>>
On Tue, 22 Oct 2024 06:31:47 GMT, David Holmes wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with six
>> additional commits since the last revision:
>>
>> - Fix comments in objectMonitor.hpp
>> - Move frame::saved_thread_address() to platform dependent files
>>
On Tue, 22 Oct 2024 06:27:26 GMT, David Holmes wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with six
>> additional commits since the last revision:
>>
>> - Fix comments in objectMonitor.hpp
>> - Move frame::saved_thread_address() to platform dependent files
>>
On Tue, 22 Oct 2024 13:51:26 GMT, Axel Boldt-Christmas
wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with six
>> additional commits since the last revision:
>>
>> - Fix comments in objectMonitor.hpp
>> - Move frame::saved_thread_address() to platform dependent f
30 matches
Mail list logo
