RFR: 8265753: Remove manual JavaThread transitions to blocked [v5]

[Robbin Ehn]

On Thu, 20 May 2021 21:51:31 GMT, David Holmes <dholmes at openjdk.org> 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 seven additional commits since the last revision:
>> 
>>  - Review fixes 2
>>  - Merge branch 'master' into 8265753
>>  - Review fixes
>>  - Merge branch 'master' into 8265753
>>  - Fixes for Dan
>>  - Merge branch 'master' into 8265753
>>  - Removed manual transitions
>
> src/hotspot/share/prims/jvmtiRawMonitor.cpp line 379:
> 
>> 377:   ret = simple_wait(self, millis);
>> 378: 
>> 379:   // Now we need to re-enter the monitor. For JavaThread's
> 
> My bad: no apostrophe in JavaThreads

Fixed

> src/hotspot/share/runtime/interfaceSupport.inline.hpp line 212:
> 
>> 210:     _thread->frame_anchor()->make_walkable(_thread);
>> 211:     OrderAccess::storestore();
>> 212:     _thread->set_thread_state(_thread_in_native);
> 
> Can't help but think the ppc/aarch64 folk have it right and that set_thread_state should always (?) have release semantics - and would then be renamed release_set_thread_state(). Also avoids a double barrier on ppc/aarch64. Followup RFE?
> But note that we need the storestore after all the make_walkable's that are followed by set_thread_state.

Ah, you mean the pre-exiting missing of a storestore in TBIVM? Fixed.
But we have more of those.

Note that on TSO it's not needed, since both are volatile and will not be re-ordered by compiler.
Yes, the platform which actually do care about avoiding release for performance have it (because it's needed).
So yes it should always be there, good RFE!

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

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