On Wed, 7 Apr 2021 16:25:28 GMT, Robbin Ehn wrote:
>>> Hi Robbin,
>>> I'm looking at it. I have a concern that it can significantly impact Loom
>>> implementation. There is a non-trivial update in Loom to support virtual
>>> threads in JVMTI Suspend/Resume. So we need to make sure your approach
On Wed, 7 Apr 2021 15:48:55 GMT, Patricio Chilano Mateo
wrote:
>> The test worked because we didn't check for suspend when leaving a safepoint
>> request back to native.
>> @pchilano have more info on why the test even works today.
>
> Right, the reason the test was working so far but not anymo
On Wed, 7 Apr 2021 22:44:41 GMT, Serguei Spitsyn wrote:
>> Hi @sspitsyn,
>>
>> I rather like the much simpler agent code and it puts the majority of logic
>> in the .java file so the reader/reviewer doesn't have to jump back and forth
>> between the two files nearly as much.
>>
>> The `id` argu
On Wed, 7 Apr 2021 19:17:33 GMT, Daniel D. Daugherty wrote:
>> Hi Dan,
>> It looks good in general.
>> But I think moving JVMTI error code checks to Java is a step in a wrong
>> direction.
>> First, it makes it inconsistent with all existing JVMTI tests. In this
>> particular case, all the comp
On Wed, 7 Apr 2021 17:52:16 GMT, Serguei Spitsyn wrote:
>> Daniel D. Daugherty has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address lyndseyBeil, robehn and sspitsyn CR comments.
>
> Hi Dan,
> It looks good in general.
> But I think mo
On Thu, 1 Apr 2021 19:46:38 GMT, Daniel D. Daugherty wrote:
>> Add three tests from JDK-4413752 ported to JVM/TI:
>>
>> - RawMonitorEnter() with SuspendThread()
>> -
>> test/hotspot/jtreg/serviceability/jvmti/SuspendWithRawMonitorEnter/SuspendWithRawMonitorEnter.java
>> -
>> test/hotspot/j
On Wed, 7 Apr 2021 16:23:08 GMT, Robbin Ehn wrote:
>> Hi Robbin,
>> I'm looking at it. I have a concern that it can significantly impact Loom
>> implementation. There is a non-trivial update in Loom to support virtual
>> threads in JVMTI Suspend/Resume. So we need to make sure your approach is
On Tue, 6 Apr 2021 18:53:54 GMT, Serguei Spitsyn wrote:
> Hi Robbin,
> I'm looking at it. I have a concern that it can significantly impact Loom
> implementation. There is a non-trivial update in Loom to support virtual
> threads in JVMTI Suspend/Resume. So we need to make sure your approach is
On Wed, 31 Mar 2021 05:52:14 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 7 Apr 2021 14:33:00 GMT, Robbin Ehn wrote:
>> test/hotspot/jtreg/serviceability/jvmti/SuspendWithCurrentThread/SuspendWithCurrentThread.java
>> line 132:
>>
>>> 130: ThreadToSuspend.setAllThreadsReady();
>>> 131:
>>> 132: while (!checkSuspendedStatus()) {
>>
>>
On Wed, 7 Apr 2021 11:43:15 GMT, Robbin Ehn wrote:
>> Daniel D. Daugherty has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address lyndseyBeil, robehn and sspitsyn CR comments.
>
> I really like simple agent code 👍
>
> Thanks!
@robehn
On Wed, 7 Apr 2021 15:40:58 GMT, Daniel D. Daugherty wrote:
>> Harold Seigel has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> remove unneeded statement
>
> Marked as reviewed by dcubed (Reviewer).
Thumbs up.
-
PR: https://g
On Wed, 7 Apr 2021 14:19:50 GMT, Harold Seigel wrote:
>> Please review this additional cleanup of use of TRAPS in hotspot runtime
>> code. The changes were tested with Mach5 tiers 1-2 on Linux, Mac OS, and
>> Windows and Mach5 tiers 3-5 on Linux x64.
>>
>> Thanks, Harold
>
> Harold Seigel has
> Hi, Please review
>
> Added jcmd option for dumping CDS archive during application runtime.
> Before this change, user has to dump shared archive in two steps: first run
> application with
> `java -XX:DumpLoadedClassList= `
> to collect shareable class names and saved in file `` ,
On Wed, 7 Apr 2021 14:31:05 GMT, Richard Reingruber wrote:
>> I removed that line, not sure if the comment needs additional fixing.
>
> I think David requested updating of the comment for
> `JavaThread::java_suspend_self_with_safepoint_check()`.
>
> I hope to delete the whole method soon and bu
On Thu, 1 Apr 2021 03:21:55 GMT, Patricio Chilano Mateo
wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into S
On Wed, 31 Mar 2021 06:57:16 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 7 Apr 2021 13:33:45 GMT, Robbin Ehn wrote:
>> src/hotspot/share/runtime/thread.cpp line 2102:
>>
>>> 2100: do {
>>> 2101: set_thread_state(_thread_blocked);
>>> 2102: // Check if _external_suspend was set in the previous loop
>>> iteration.
>>
>> Note that the comment for thi
On Wed, 31 Mar 2021 06:53:50 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into SuspendInHa
On Mon, 5 Apr 2021 23:15:48 GMT, David Holmes wrote:
>> Harold Seigel has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> remove unneeded statement
>
> src/hotspot/share/classfile/klassFactory.cpp line 117:
>
>> 115:
On Mon, 5 Apr 2021 23:28:38 GMT, David Holmes wrote:
>> Harold Seigel has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Undo change to ObjectSynchronizer::jni_exit()
>
> Hi Harold,
>
> Lots of good cleanup here but also a couple of things
On Mon, 5 Apr 2021 19:08:38 GMT, Patricio Chilano Mateo
wrote:
>> Harold Seigel has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> remove unneeded statement
>
> src/hotspot/share/runtime/synchronizer.cpp line 609:
>
>> 607: // intention
> Please review this additional cleanup of use of TRAPS in hotspot runtime
> code. The changes were tested with Mach5 tiers 1-2 on Linux, Mac OS, and
> Windows and Mach5 tiers 3-5 on Linux x64.
>
> Thanks, Harold
Harold Seigel has updated the pull request incrementally with one additional
com
On Wed, 31 Mar 2021 06:51:19 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 31 Mar 2021 06:50:17 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into SuspendInHa
On Wed, 31 Mar 2021 06:46:00 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 31 Mar 2021 06:41:20 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into SuspendInHa
On Wed, 31 Mar 2021 09:37:37 GMT, Richard Reingruber wrote:
>> src/hotspot/share/runtime/thread.cpp line 2105:
>>
>>> 2103: if (is_external_suspend()) {
>>> 2104: java_suspend_self();
>>> 2105: }
>>
>> It is not at all clear to me how this method should interact with thread
>> su
On Wed, 31 Mar 2021 06:37:00 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Tue, 6 Apr 2021 18:37:38 GMT, Patricio Chilano Mateo
wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into S
On Wed, 7 Apr 2021 13:17:40 GMT, Robbin Ehn wrote:
>> src/hotspot/share/runtime/handshake.hpp line 122:
>>
>>> 120: // must take slow path, process_by_self(), if _lock is held.
>>> 121: bool should_process() {
>>> 122: return !_queue.is_empty() || _lock.is_locked();
>>
>> While trying t
On Wed, 7 Apr 2021 13:08:47 GMT, Richard Reingruber wrote:
>> Today the ThreadBlockInVM tbivm(current); is inside scope of
>> JavaThreadBlockedOnMonitorEnterState jtbmes(current, this);.
>> So this can happen today also.
>>
>> If you are context switch just before
>> current->set_current_pendi
On Wed, 31 Mar 2021 06:27:38 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 31 Mar 2021 06:27:12 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into SuspendInHa
On Wed, 7 Apr 2021 12:04:42 GMT, Robbin Ehn wrote:
> Today the ThreadBlockInVM tbivm(current); is inside scope of
> JavaThreadBlockedOnMonitorEnterState jtbmes(current, this);.
> So this can happen today also.
>
> If you are context switch just before
> current->set_current_pending_monitor(NUL
On Wed, 31 Mar 2021 05:53:43 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 31 Mar 2021 05:44:35 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
> Please review this additional cleanup of use of TRAPS in hotspot runtime
> code. The changes were tested with Mach5 tiers 1-2 on Linux, Mac OS, and
> Windows and Mach5 tiers 3-5 on Linux x64.
>
> Thanks, Harold
Harold Seigel has updated the pull request incrementally with one additional
com
On Wed, 7 Apr 2021 11:50:09 GMT, Robbin Ehn wrote:
>> src/hotspot/share/runtime/objectMonitor.cpp line 428:
>>
>>> 426: } else {
>>> 427: // Only exit path from for loop
>>> 428: SafepointMechanism::process_if_requested(current);
>>
>> I think the current thread can suspen
On Wed, 7 Apr 2021 09:56:19 GMT, Richard Reingruber wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into Suspen
On Thu, 1 Apr 2021 19:46:38 GMT, Daniel D. Daugherty wrote:
>> Add three tests from JDK-4413752 ported to JVM/TI:
>>
>> - RawMonitorEnter() with SuspendThread()
>> -
>> test/hotspot/jtreg/serviceability/jvmti/SuspendWithRawMonitorEnter/SuspendWithRawMonitorEnter.java
>> -
>> test/hotspot/j
On Wed, 7 Apr 2021 06:51:22 GMT, Robbin Ehn wrote:
>> But I still think we can just call is_suspended() here. If is_suspended()
>> returns false, then Z hasn't finished suspending A yet (either async
>> operation was still not added to the queue in case this is the first suspend
>> for A, or d
On Wed, 31 Mar 2021 08:27:35 GMT, Robbin Ehn wrote:
>> A suspend request is done by handshaking thread target thread(s). When
>> executing the handshake operation we know the target mutator thread is in a
>> dormant state (as in safepoint safe state). We have a guarantee that it will
>> check
On 7/04/2021 6:37 pm, Robbin Ehn wrote:
On Wed, 31 Mar 2021 06:08:49 GMT, David Holmes wrote:
Robbin Ehn has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains two commits:
- Merge branch 'master' into SuspendInHandshake
- 8257831: S
On 7/04/2021 6:06 pm, Robbin Ehn wrote:
On Wed, 7 Apr 2021 07:04:50 GMT, Robbin Ehn wrote:
I'm also unclear what Robbin is referring to. I go back to my original comment
that surely JVMTI_ERROR_THREAD_NOT_ALIVE is impossible here?
We do a callback with a terminated thread, if the thread the
On Wed, 31 Mar 2021 05:57:30 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 31 Mar 2021 06:08:49 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Tue, 30 Mar 2021 15:50:56 GMT, Richard Reingruber wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (r
On Wed, 7 Apr 2021 07:04:50 GMT, Robbin Ehn wrote:
>> I'm also unclear what Robbin is referring to. I go back to my original
>> comment that surely JVMTI_ERROR_THREAD_NOT_ALIVE is impossible here?
>
> We do a callback with a terminated thread, if the thread then in the agent
> tries to suspend
On 7/04/2021 5:37 pm, Robbin Ehn wrote:
On Wed, 31 Mar 2021 05:38:14 GMT, David Holmes wrote:
Robbin Ehn has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains two commits:
- Merge branch 'master' into SuspendInHandshake
- 8257831: S
On Wed, 31 Mar 2021 05:38:14 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On 7/04/2021 5:08 pm, Robbin Ehn wrote:
On Wed, 7 Apr 2021 04:22:21 GMT, David Holmes wrote:
By "treat this the same way as the others", you mean check and return either
JVMTI_ERROR_THREAD_NOT_ALIVE or JVMTI_ERROR_THREAD_SUSPENDED as
appropriate when we get a false back from JvmtiSuspendContro
On Wed, 31 Mar 2021 05:03:47 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - Merge branch 'master' into SuspendInHa
On Wed, 31 Mar 2021 05:21:03 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 31 Mar 2021 04:56:32 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 31 Mar 2021 03:59:35 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains two commits:
>>
>> - Merge branch 'master' into SuspendInHandshake
>> - 8257831: Suspend with handshake (review
On Wed, 7 Apr 2021 04:22:21 GMT, David Holmes wrote:
>> By "treat this the same way as the others", you mean check and return either
>> JVMTI_ERROR_THREAD_NOT_ALIVE or JVMTI_ERROR_THREAD_SUSPENDED as
>> appropriate when we get a false back from
>> JvmtiSuspendControl::suspend(current)?
>>
>> I'
57 matches
Mail list logo
