RFR: 8254264: Remove redundant cross_modify_fence()

[Patricio Chilano]

Hi David,

On 10/15/20 5:22 AM, David Holmes wrote:
> Hi Patricio,
>
> On 15/10/2020 1:20 am, Patricio Chilano Mateo wrote:
>> Hi all,
>>
>> Please review the following patch that removes some uneeded uses of 
>> cross_modify_fence() in common code, in particular
>> from ~ThreadBlockInVM(), ~ThreadBlockInVMWithDeadlockCheck() and 
>> java_suspend_self_with_safepoint_check(). These fences
>> were added before each JavaThread had to disarm itself (8230594). 
>> After a safepoint/handshake each JavaThread will
>> always call SafepointMechanism::process_if_requested_slow() when 
>> transitioning out of the safe state and will execute a
>> cross_modify_fence(). Tested with mach5 tiers1-7.
>
> As you have already done the analysis, could you show the exact call 
> sequences that allow for the removal of the code as described? I'm 
> trying to verify what you've stated but trying to reverse engineer the 
> calling sequences is painful. :)
So, before each JavaThread had to disarm itself you could have the 
following sequence:

- JT T creates ThreadBlockInVM object and then blocks (usually in 
park()/wait())
- VMThread arms T for a safepoint operation. Since T is in 
_thread_blocked state it's safe so safepoint starts.
- Once safepoint operation is done VMThread disarms T
- T wakes up and executes ~ThreadBlockInVM(). Since the poll word/page 
is not armed it just continues execution. If we didn't have that 
cross_modify_fence() in ~ThreadBlockInVM() we can transition back to 
java later without executing the fence instruction.

Same sequence with ~ThreadBlockInVMWithDeadlockCheck and 
java_suspend_self_with_safepoint_check(). So when transitioning out of 
the _thread_blocked state a JT wouldn't know if a safepoint happened so 
we had that fence there unconditionally.

Today disarming of the poll word/page is done by each JT, so if a 
safepoint happened while the JT was blocked the poll word/page will 
remain armed. The JT when transitioning out of the _thread_blocked will 
call SafepointMechanism::process_if_requested() (in 
~TBIVM/~TBIVMWDC/java_suspend_self_with_safepoint_check()), will go 
through process_if_requested_slow() to check if it needs to process some 
operation and at the end will execute a cross_modify_fence().


Patricio
> Thanks,
> David
>
>> Thanks,
>> Patricio
>>
>> -------------
>>
>> Commit messages:
>>   - v1
>>
>> Changes: https://git.openjdk.java.net/jdk/pull/655/files
>>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=655&range=00
>>    Issue: https://bugs.openjdk.java.net/browse/JDK-8254264
>>    Stats: 5 lines in 2 files changed: 0 ins; 5 del; 0 mod
>>    Patch: https://git.openjdk.java.net/jdk/pull/655.diff
>>    Fetch: git fetch https://git.openjdk.java.net/jdk 
>> pull/655/head:pull/655
>>
>> PR: https://git.openjdk.java.net/jdk/pull/655
>>