On Wed, 23 Sep 2020 02:45:27 GMT, David Holmes <dhol...@openjdk.org> 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<HandshakeOperation*> _queue; >> 77: Mutex _lock; >> 78: Thread* _active_handshaker; > > To be clear, the _handshakee is the always the target JavaThread, while the > _active_handshaker, is the thread that is > actually executing the handshake operation (ie do_thread). If so can you add > comments on these declarations to clarify > that. Thanks. Fixed > src/hotspot/share/runtime/handshake.hpp line 90: > >> 88: void add_operation(HandshakeOperation* op); >> 89: HandshakeOperation* pop_for_self(); >> 90: HandshakeOperation* pop_for_processor(); > > What is "processor" in this context - the active handshaker? Can we not > introduce yet another piece of terminology > here. We should have consistency of naming when it comes to "self" and > others. ie. we have > pop_for_self() > > but > > has_operation() > > rather than > > has_operation_for_self() > > If we made the "self" case explicit then we could leave the not-self case > implicit e.g. > > pop_for_self(); // Called by handshakee only > pop(); // Called by handshaker or VMThread > has_operation_for_self(); // Is there an operation that can be executed by > the handshakee itself > has_operation(); // Is there an operation that can be executed by > the handshaker or VMThread > We can then stop using "processor" in other places as well. Fixing ------------- PR: https://git.openjdk.java.net/jdk/pull/151
