On Tue, 30 Mar 2021 13:54:58 GMT, Richard Reingruber <rr...@openjdk.org> 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 baseline)
>
> src/hotspot/share/prims/jvmtiRawMonitor.cpp line 364:
> 
>> 362:     for (;;) {
>> 363:       simple_enter(jt);
>> 364:       if (!SafepointMechanism::should_process(jt)) {
> 
> It seems to be likely that the condition is false in the first loop iteration 
> and the thread has to do another iteration even if not suspended. Wouldn't it 
> be ok to break from the loop if `!jt->is_suspended()`?
> I'm asking because in ObjectMonitor::enter() L414 there is similar code and 
> the condition there is `SafepointMechanism::should_process(current) && 
> current->suspend_request_pending()`

I didn't add that optimization here because I don't think it is needed, but I 
can add it.

> src/hotspot/share/runtime/handshake.cpp line 486:
> 
>> 484:         // Asynchronous may block so they may not execute 
>> ~PreserveExceptionMark before safepointing
>> 485:         // in outer loop.
>> 486:         op->do_handshake(_handshakee);
> 
> Maybe add PauseNoSafepointVerifier to document that the current thread can 
> transition to a safe state?

Fixed

-------------

PR: https://git.openjdk.java.net/jdk/pull/3191

Reply via email to