On Tue, 29 Sep 2020 06:25:40 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev
>> excludes the unrelated changes brought in by the merge/rebase. The pull
>> request contains 11 additional commits
On Mon, 28 Sep 2020 09:01:22 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Mon, 28 Sep 2020 09:01:22 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Mon, 28 Sep 2020 19:15:35 GMT, Robbin Ehn wrote:
>> Robbin, thank you for your answers!
>> There are JVMTI functions that are specified to return error code
>> JVMTI_ERROR_THREAD_NOT_ALIVE.
>> As an example, see:
>>
>>
On Mon, 28 Sep 2020 18:20:58 GMT, Serguei Spitsyn wrote:
> Robbin, thank you for your answers!
> There are JVMTI functions that are specified to return error code
> JVMTI_ERROR_THREAD_NOT_ALIVE.
> As an example, see:
>
On Mon, 28 Sep 2020 08:40:15 GMT, Robbin Ehn wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add constructor and comment. Previous renames caused confusing, improved
>> names once more and moved non-public to
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> Since we have several use-cases for them we can have
On Thu, 24 Sep 2020 08:18:12 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Mon, 28 Sep 2020 05:28:43 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add constructor and comment. Previous renames caused confusing, improved
>> names once more and moved non-public to
On Thu, 24 Sep 2020 08:18:12 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> Since we have several use-cases for them we can have
On Thu, 24 Sep 2020 06:54:32 GMT, Robbin Ehn wrote:
>> Hi Robbin,
>> I've looked more at the Serviceability files.
>> The fix looks good in general. Nice refactoring and simplification with the
>> JvmtiHandshakeClosure.
>> It is a little unexpected that the do_thread can be executed by
>>
On Wed, 23 Sep 2020 23:45:49 GMT, Serguei Spitsyn wrote:
>> No significant comments. All my concerns relate to naming and terminology,
>> where I think there is scope for quite a bit
>> of tidying up. Thanks.
>
> Hi Robbin,
> I've looked more at the Serviceability files.
> The fix looks good in
On 23/09/2020 8:11 pm, Robbin Ehn wrote:
On Wed, 23 Sep 2020 03:04:39 GMT, David Holmes wrote:
Robbin Ehn has updated the pull request incrementally with one additional
commit since the last revision:
Update after Coleen
src/hotspot/share/runtime/handshake.cpp line 63:
61: };
62:
On 23/09/2020 7:37 pm, Robbin Ehn wrote:
On Wed, 23 Sep 2020 02:56:00 GMT, David Holmes wrote:
Robbin Ehn has updated the pull request incrementally with one additional
commit since the last revision:
Update after Coleen
src/hotspot/share/runtime/handshake.hpp line 97:
95: }
96:
On Wed, 23 Sep 2020 04:10:36 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> No significant comments. All my concerns relate to naming and terminology,
> where I think
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> Since we have several use-cases for them we can have
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> Since we have several use-cases for them we can have
On Wed, 23 Sep 2020 04:06:46 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/utilities/filterQueue.hpp line 56:
>
>> 54: }
>> 55:
>> 56: // MT-safe
>
On Wed, 23 Sep 2020 03:20:02 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/runtime/handshake.cpp line 244:
>
>> 242: // A new thread on the
On Wed, 23 Sep 2020 04:03:30 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/utilities/filterQueue.hpp line 32:
>
>> 30:
>> 31: template
>> 32: class
On Wed, 23 Sep 2020 03:04:39 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/runtime/handshake.cpp line 63:
>
>> 61: };
>> 62:
>> 63: class
On Wed, 23 Sep 2020 02:56:00 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/runtime/handshake.hpp line 97:
>
>> 95: }
>> 96: bool
On Wed, 23 Sep 2020 02:54:09 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/runtime/handshake.hpp line 96:
>
>> 94: return !_queue.is_empty();
>> 95:
On Wed, 23 Sep 2020 02:45:27 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/runtime/handshake.hpp line 78:
>
>> 76: FilterQueue _queue;
>> 77: Mutex
On Wed, 23 Sep 2020 02:41:37 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/runtime/handshake.hpp line 51:
>
>> 49: virtual ~HandshakeClosure() {}
>>
On Wed, 23 Sep 2020 02:40:31 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/runtime/handshake.hpp line 45:
>
>> 43: // a single target/direct handshake
On Mon, 21 Sep 2020 21:59:40 GMT, Coleen Phillimore wrote:
>> The thread executing this handshake operation, what the current thread is
>> doesn't matter.
>> You can't use current threads resources or be dependent otherwise on it.
>>
>> Exception being locking issues in JVM TI, where we are
On Mon, 21 Sep 2020 11:07:14 GMT, Robbin Ehn wrote:
>> src/hotspot/share/runtime/handshake.hpp line 44:
>>
>>> 42: // by the target JavaThread itself or, depending on whether the
>>> operation is
>>> 43: // a single target/direct handshake or not, by the JavaThread that
>>> requested the
>>>
On Tue, 22 Sep 2020 07:54:57 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Tue, 22 Sep 2020 07:54:57 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Tue, 22 Sep 2020 14:07:11 GMT, Robbin Ehn wrote:
>> src/hotspot/share/prims/whitebox.cpp line 2050:
>>
>>> 2048: JavaThread* target = java_lang_Thread::thread(thread_oop);
>>> 2049: TraceSelfClosure* tsc = new TraceSelfClosure(target);
>>> 2050: Handshake::execute(tsc, target);
On Tue, 22 Sep 2020 07:54:57 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Tue, 22 Sep 2020 14:22:58 GMT, Robbin Ehn wrote:
>> I don't understand, you'd have to rearrange the initializers in the
>> constructor too, but I don't see any order
>> dependance. Moving over _lock helps, so this is fine.
>
> You want a cosmetic change in the member declaration.
> I'm
On Tue, 22 Sep 2020 12:20:36 GMT, Coleen Phillimore wrote:
>> The order of members matter since C++ initialize them in declared order.
>> My opinion when changing this was that it was easier to read when passing
>> the only argument to the first member being
>> initialized, thus _handshakee
On Tue, 22 Sep 2020 12:14:19 GMT, Coleen Phillimore wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update after Coleen
>
> src/hotspot/share/prims/whitebox.cpp line 2050:
>
>> 2048: JavaThread* target =
On Tue, 22 Sep 2020 07:20:30 GMT, Robbin Ehn wrote:
>> FilterQueue _queue;
>> JavaThread* _handshakee;
>> Mutex _lock;
>> Thread* _active_handshaker;
>>
>> Isn't this nicer? (it didn't keep the formatting in the comment)
>
> The order of members matter since C++ initialize
On Tue, 22 Sep 2020 07:54:57 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> Since we have several use-cases for them we can have
On Tue, 22 Sep 2020 07:00:06 GMT, Robbin Ehn wrote:
>> Hi Robbin,
>>
>> I've gone back to refresh myself on the previous discussions and (internal)
>> design walk-throughs to get a better sense
>> of these changes. Really the "asynchronous handshake" aspect is only a small
>> part of this.
On Mon, 21 Sep 2020 21:26:08 GMT, Coleen Phillimore wrote:
>> src/hotspot/share/runtime/handshake.hpp line 78:
>>
>>> 76: FilterQueue _queue;
>>> 77: Mutex _lock;
>>> 78: Thread* _active_handshaker;
>>
>> Nit: can you line up the data member names for lock and _active_handshaker ?
>
>
On Mon, 21 Sep 2020 21:18:20 GMT, Coleen Phillimore wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev
>> excludes the unrelated changes brought in by the merge/rebase. The pull
>> request contains five additional
On Tue, 22 Sep 2020 03:12:01 GMT, David Holmes wrote:
>> Looks mostly good to me!
>
> Hi Robbin,
>
> I've gone back to refresh myself on the previous discussions and (internal)
> design walk-throughs to get a better sense
> of these changes. Really the "asynchronous handshake" aspect is only a
On Mon, 21 Sep 2020 22:57:55 GMT, Coleen Phillimore wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev
>> excludes the unrelated changes brought in by the merge/rebase. The pull
>> request contains five additional
On Mon, 21 Sep 2020 12:19:09 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Mon, 21 Sep 2020 11:02:10 GMT, Robbin Ehn wrote:
>> src/hotspot/share/runtime/handshake.cpp line 313:
>>
>>> 311: }
>>> 312:
>>> 313: int executed_by_driver = 0;
>>
>> Again why driver?? Isn't it either the current thread or the target that
>> will execute the op?
>
> The thread
On Mon, 21 Sep 2020 12:19:09 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Mon, 21 Sep 2020 21:19:32 GMT, Coleen Phillimore wrote:
>> Robbin Ehn has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev
>> excludes the unrelated changes brought in by the merge/rebase. The pull
>> request contains five additional
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> Since we have several use-cases for them we can have
On Mon, 21 Sep 2020 05:42:29 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Mon, 21 Sep 2020 05:31:59 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed double checks
>> Added NSV
>> ProcessResult to enum
>> Fixed logging
>> Moved _active_handshaker to
On Mon, 21 Sep 2020 05:26:08 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Mon, 21 Sep 2020 05:18:06 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Mon, 21 Sep 2020 05:17:03 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed double checks
>> Added NSV
>> ProcessResult to enum
>> Fixed logging
>> Moved _active_handshaker to
On Mon, 21 Sep 2020 05:01:29 GMT, David Holmes wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Mon, 21 Sep 2020 06:13:55 GMT, David Holmes wrote:
> There is still a lack of motivation for this feature in the JBS issue. What
> kind of handshakes need to be asynchronous?
> Any async operation implies that the requester doesn't care about when or
> even if, the operation gets executed -
On Mon, 21 Sep 2020 05:27:00 GMT, David Holmes wrote:
>> src/hotspot/share/runtime/handshake.hpp line 45:
>>
>>> 43: // a single target/direct handshake or not, by the JavaThread that
>>> requested the
>>> 44: // handshake or the VMThread respectively.
>>> 45: class HandshakeClosure : public
On Fri, 18 Sep 2020 20:52:49 GMT, Daniel D. Daugherty
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Mon, 21 Sep 2020 05:04:45 GMT, David Holmes wrote:
>> src/hotspot/share/prims/whitebox.cpp line 2032:
>>
>>> 2030: void do_thread(Thread* th) {
>>> 2031: assert(th->is_Java_thread(), "sanity");
>>> 2032: JavaThread* jt = (JavaThread*)th;
>>
>> Can whitebox.cpp code use the
On Fri, 18 Sep 2020 20:37:10 GMT, Daniel D. Daugherty
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Mon, 21 Sep 2020 05:32:54 GMT, David Holmes wrote:
>> src/hotspot/share/runtime/interfaceSupport.inline.hpp line 157:
>>
>>> 155:
>>> 156: // Threads shouldn't block if they are in the middle of printing,
>>> but...
>>> 157:
On Fri, 18 Sep 2020 20:21:09 GMT, Daniel D. Daugherty
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Fri, 18 Sep 2020 20:07:47 GMT, Daniel D. Daugherty
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Fri, 18 Sep 2020 20:40:42 GMT, Daniel D. Daugherty
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Fri, 18 Sep 2020 20:01:02 GMT, Daniel D. Daugherty
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed double checks
>> Added NSV
>> ProcessResult to enum
>> Fixed logging
>> Moved _active_handshaker
On Fri, 18 Sep 2020 19:46:34 GMT, Daniel D. Daugherty
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
Correction ...
On 21/09/2020 4:16 pm, David Holmes wrote:
On Thu, 17 Sep 2020 12:07:15 GMT, Robbin Ehn wrote:
This patch implements asynchronous handshake, which changes how handshakes
works by default. Asynchronous handshakes
are target only executed, which they may never be executed.
On Fri, 18 Sep 2020 20:51:17 GMT, Daniel D. Daugherty
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removed double check, fix comment, removed not needed function, updated
>> logs
>
>
On Thu, 17 Sep 2020 19:51:24 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Thu, 17 Sep 2020 12:07:15 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Thu, 17 Sep 2020 19:51:24 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Thu, 17 Sep 2020 12:07:15 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Thu, 17 Sep 2020 19:51:24 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Thu, 17 Sep 2020 18:57:33 GMT, Patricio Chilano Mateo
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed double checks
>> Added NSV
>> ProcessResult to enum
>> Fixed logging
>> Moved
On Thu, 17 Sep 2020 19:23:09 GMT, Robbin Ehn wrote:
>> src/hotspot/share/runtime/handshake.cpp line 508:
>>
>>> 506: assert(op->_target == NULL || _handshakee == op->_target, "Wrong
>>> thread");
>>> 507: log_trace(handshake)("Processing handshake " INTPTR_FORMAT " by
>>> %s",
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> Since we have several use-cases for them we can have
On Thu, 17 Sep 2020 18:28:20 GMT, Patricio Chilano Mateo
wrote:
>> Robbin Ehn has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fixed double checks
>> Added NSV
>> ProcessResult to enum
>> Fixed logging
>> Moved
On Thu, 17 Sep 2020 12:07:15 GMT, Robbin Ehn wrote:
>> This patch implements asynchronous handshake, which changes how handshakes
>> works by default. Asynchronous handshakes
>> are target only executed, which they may never be executed. (target may
>> block on socket for the rest of VM
On Mon, 14 Sep 2020 13:00:59 GMT, Robbin Ehn wrote:
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> This patch implements asynchronous handshake, which changes how handshakes
> works by default. Asynchronous handshakes
> are target only executed, which they may never be executed. (target may block
> on socket for the rest of VM lifetime)
> Since we have several use-cases for them we can have
Hi Robbin,
Changes look good to me! Some minor comments:
src/hotspot/share/prims/jvmtiThreadState.cpp
222: assert(current_thread == get_thread() ||
223: SafepointSynchronize::is_at_safepoint() ||
224: get_thread()->is_handshake_safe_for(current_thread),
225: "call by myself
This patch implements asynchronous handshake, which changes how handshakes
works by default. Asynchronous handshakes
are target only executed, which they may never be executed. (target may block
on socket for the rest of VM lifetime)
Since we have several use-cases for them we can have many
82 matches
Mail list logo
