On Wed, 12 May 2021 08:04:47 GMT, Robbin Ehn <r...@openjdk.org> wrote:

>> Please consider this change which removes the manual transitions to blocked.
>> This adds a preprocess template/functor which is executed in the destructor 
>> of 'ThreadBlockInVM' if we are going to do any processing.
>> This gives us a way to backout of the object/raw monitor before suspend or 
>> other processing, such as a safepoint.
>> 
>> The object monitor code could be straight forward changed to use this 
>> instead of manual transitions.
>> 
>> Raw monitors on the other hand are a bit more complicated due to 'implicit' 
>> rules (consequences of the specs).
>> Added a comment in hpp trying to explain it; we cannot simply transition 
>> with a raw monitor held.
>> This caused the change in the destructor ~ThreadInVMfromNative() (this 
>> specific change have also been tested in unrelated exploration of 
>> transition), now this RAII does the same as we do when going to native from 
>> Java, just setting the state.
>> Since we are going from an unsafe state, in VM, to a safe state, in native, 
>> we do not need to check the poll.
>> That made it possible to careful use ThreadInVMfromNative in raw monitors.
>> 
>> I also remove the early CAS in raw_enter.
>> We lock a lock to do a CAS, in the uncontended case means CAS on lock then 
>> CAS raw monitor.
>> Now we instead do a transitions, in the uncontended case means fence, CAS 
>> raw monitor, fence.
>> (multiple fence (CAS is also a fence) very close to each other have little 
>> additional performance impact on contemporary hardware)
>> 
>> Passes t1-t7 and manual stressing relevant test groups.
>
> Robbin Ehn has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixes for Dan

src/hotspot/share/prims/jvmtiRawMonitor.cpp line 395:

> 393:           ThreadBlockInVMPreprocess<ExitOnSuspend> tbivmp(jt, eos);
> 394:           simple_enter(jt);
> 395:           _recursions = save;

I think you should restore `_recursions` only when finally leaving the loop 
because if you do it here and exit the monitor again because of a suspend then 
another thread could hit the `guarantee(_recursions == 0, "invariant")` at 
L349. This is a preexisting issue but since you're modifying this I reckon it 
makes sense to correct it.

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

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

Reply via email to