On Tue, 5 Nov 2024 06:30:55 GMT, Fei Yang wrote:
>> Great, thanks Dean. I removed `possibly_adjust_frame()` and the related code.
>> @RealFYang I made the equivalent change for riscv, could you verify it's
>> okay?
>
> @pchilano : Hi, Great to see `possibly_adjust_frame()` go away. Nice cleanup!
On Mon, 4 Nov 2024 18:18:38 GMT, Patricio Chilano Mateo
wrote:
>> Here's my suggested C2 change:
>>
>> diff --git a/src/hotspot/cpu/aarch64/aarch64.ad
>> b/src/hotspot/cpu/aarch64/aarch64.ad
>> index d9c77a2f529..1e99db191ae 100644
>> --- a/src/hotspot/cpu/aarch64/aarch64.ad
>> +++ b/src/hotsp
On Fri, 1 Nov 2024 20:08:51 GMT, Dean Long wrote:
>> It turns out if we try to set last pc to the instruction after the
>> adjustment, then we need an oopmap there, and that would require more C2
>> changes. Then I thought about restoring SP from FP or last_Java_fp, but I
>> don't think we ca
On Fri, 1 Nov 2024 07:14:35 GMT, Dean Long wrote:
>>> OK, so you're saying it's the stack adjustment that's the problem. It
>>> sounds like there is code that is using rsp instead of last_Java_sp to
>>> compute the frame boundary. Isn't that a bug that should be fixed?
>>>
>> It's not a bug, it
On Thu, 31 Oct 2024 16:27:05 GMT, Patricio Chilano Mateo
wrote:
>> OK, so you're saying it's the stack adjustment that's the problem. It
>> sounds like there is code that is using rsp instead of last_Java_sp to
>> compute the frame boundary. Isn't that a bug that should be fixed? I also
>>
On Thu, 31 Oct 2024 02:33:30 GMT, Dean Long wrote:
> OK, so you're saying it's the stack adjustment that's the problem. It sounds
> like there is code that is using rsp instead of last_Java_sp to compute the
> frame boundary. Isn't that a bug that should be fixed?
>
It's not a bug, it's just th
On Tue, 29 Oct 2024 19:01:03 GMT, Patricio Chilano Mateo
wrote:
>>> One way to get rid of this would be to have c2 just set last_Java_pc too
>>> along with last_Java_sp, so we don't need to push lr to be able to do
>>> last_Java_sp[-1] to make the frame walkable.
>>
>> If that would solve the
On Mon, 28 Oct 2024 21:07:47 GMT, Dean Long wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Objec
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 Mon, 28 Oct 2024 23:38:43 GMT, Dean Long wrote:
>>> Could the problem be solved with a resume adapter instead, like the
>>> interpreter uses?
>>>
>> It will just move the task of adjusting the size of the frame somewhere else.
>
>> One way to get rid of this would be to have c2 just set last_
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 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 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 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 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 Mon, 28 Oct 2024 18:56:58 GMT, Patricio Chilano Mateo
wrote:
>> The issue with the c2 runtime stub on aarch64 (and riscv) is that
>> cb->frame_size() doesn't match the size of the physical frame, it's short by
>> 2 words. I explained the reason for that in the comment above. So for a
>> re
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 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 01:13:05 GMT, David Holmes wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Ob
On Mon, 28 Oct 2024 19:45:08 GMT, Dean Long wrote:
> If preemption was cancelled, we skip over the cleanup.
>
We only skip the cleanup for the enterSpecial frame since we are going to call
thaw again, all other frames are removed:
https://github.com/openjdk/jdk/pull/21565/files#diff-b938ab8a7bd
On Fri, 25 Oct 2024 21:33:24 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 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 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 Mon, 28 Oct 2024 10:37:21 GMT, Yudi Zheng wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Obje
On Mon, 28 Oct 2024 07:55:02 GMT, Richard Reingruber wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional
On Mon, 28 Oct 2024 17:30:44 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp line 159:
>>
>>> 157:
>>> 158: // The interpreter native wrapper code adds space in the stack equal
>>> to size_of_parameters()
>>> 159: // after the fixed
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 Sat, 26 Oct 2024 07:04:28 GMT, Richard Reingruber wrote:
>> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 3796:
>>
>>> 3794: __ movbool(rscratch1, Address(r15_thread,
>>> JavaThread::preemption_cancelled_offset()));
>>> 3795: __ testbool(rscratch1);
>>> 3796: __ jcc(Assembler::notZ
On Sat, 26 Oct 2024 06:56:50 GMT, Richard Reingruber wrote:
>> src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1567:
>>
>>> 1565:
>>> 1566: // In case of preemption, this is where we will resume once we
>>> finally acquire the monitor.
>>> 1567: bind(resume_pc);
>>
>> If the idea is
On Mon, 28 Oct 2024 18:51:31 GMT, Dean Long wrote:
>> Its indeed difficult to see how the value is propagaged. I think it goes
>> like this:
>>
>> - read from the frame anchor and set as pc of `_last_frame`:
>> https://github.com/pchilano/jdk/blob/66d5385f8a1c84e73cdbf385239089a7a9932a9e/src/h
On Sat, 26 Oct 2024 01:58:30 GMT, Dean Long wrote:
>> src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp line 310:
>>
>>> 308: sp -= 2;
>>> 309: sp[-2] = sp[0];
>>> 310: sp[-1] = sp[1];
>>
>> This also seems fragile. This seems to depend on an intimate knowledge of
On Mon, 28 Oct 2024 18:56:25 GMT, Patricio Chilano Mateo
wrote:
>> Could the problem be solved with a resume adapter instead, like the
>> interpreter uses?
>
> The issue with the c2 runtime stub on aarch64 (and riscv) is that
> cb->frame_size() doesn't match the size of the physical frame, it'
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 Sat, 26 Oct 2024 06:51:08 GMT, Richard Reingruber wrote:
>> src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1555:
>>
>>> 1553: // Make VM call. In case of preemption set last_pc to the one we
>>> want to resume to.
>>> 1554: adr(rscratch1, resume_pc);
>>> 1555: str(rscratch1, Addr
On Sat, 26 Oct 2024 00:30:25 GMT, Dean Long wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Objec
On Sat, 26 Oct 2024 00:17:33 GMT, Dean Long wrote:
>It sounds like freeze/thaw isn't preserving FP, even though it is a
>callee-saved register according to the ABI. If the stubs tried to modify FP
>(or any other callee-saved register) and use that value after the native call,
>wouldn't that be
On Fri, 25 Oct 2024 21:57:01 GMT, Coleen Phillimore wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional
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 Fri, 25 Oct 2024 21:33:24 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 Sat, 26 Oct 2024 01:51:12 GMT, Dean Long wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Objec
On Fri, 25 Oct 2024 21:33:24 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 Mon, 28 Oct 2024 13:08:37 GMT, Richard Reingruber wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional
On Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 13:11:18 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1550:
>>
>>> 1548: #endif /* ASSERT */
>>> 1549:
>>> 1550: push_cont_fastpath();
>>
>> One of the callers of this gives a clue what it does.
>>
>> __ push_cont_fastpath
On Fri, 25 Oct 2024 21:33:24 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 Sat, 26 Oct 2024 01:54:26 GMT, Dean Long wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Objec
On Sat, 26 Oct 2024 01:42:17 GMT, Dean Long wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Objec
On Sat, 26 Oct 2024 01:40:41 GMT, Dean Long wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Objec
On Fri, 25 Oct 2024 21:33:24 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 Sat, 26 Oct 2024 00:27:25 GMT, Dean Long wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Restore use of atPointA in test StopThreadTest.java
>> - remove interruptible check from conditional in Objec
On Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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, 22 Oct 2024 02:18:19 GMT, Patricio Chilano Mateo
wrote:
>> src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp line 300:
>>
>>> 298: CodeBlob* cb = top.cb();
>>> 299:
>>> 300: if (cb->frame_size() == 2) {
>>
>> Is this a filter to identify c2 runtime stubs? Is there
On Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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 Fri, 25 Oct 2024 21:33:24 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
66 matches
Mail list logo
