RFR(s): 8234086: VM operation can be simplified

[David Holmes]

Hi Robbin,

On 21/11/2019 9:50 pm, Robbin Ehn wrote:
> Hi,
> 
> Here is v3:
> 
> Full:
> http://cr.openjdk.java.net/~rehn/8234086/v3/full/webrev/

src/hotspot/share/runtime/synchronizer.cpp

Looking at the highly discussed:

if (Atomic::load(&ForceMonitorScavenge) == 0 && Atomic::xchg (1, 
&ForceMonitorScavenge) == 0) {

why isn't that just:

if (Atomic::cmpxchg(1, &ForceMonitorScavenge,0) == 0) {

??

Also while we are here can we clean this up further:

static volatile int ForceMonitorScavenge = 0;

becomes

static int _forceMonitorScavenge = 0;

so the variable doesn't look like it came from globals.hpp :)

Just to be clear, I understand the changes around monitor scavenging 
now, though I'm not sure getting rid of async VM ops and replacing with 
a new way to directly wakeup the VMThread really amounts to a 
simplification.

---

src/hotspot/share/runtime/vmOperations.hpp

I still think getting rid of Mode altogether would be a good 
simplification. :)

Thanks,
David
-----


> Inc:
> http://cr.openjdk.java.net/~rehn/8234086/v3/inc/webrev/
> 
> Tested t1-3
> 
> Thanks, Robbin
> 
> On 2019-11-19 12:05, Robbin Ehn wrote:
>> Hi all, please review.
>>
>> CMS was the last real user of the more advantage features of VM 
>> operation.
>> VM operation can be simplified to always be an stack object and thus 
>> either be
>> of safepoint or no safepoint type.
>>
>> VM_EnableBiasedLocking is executed once by watcher thread, if needed 
>> (default not used). Making it synchrone doesn't matter.
>> VM_ThreadStop is executed by a JavaThread, that thread should stop for 
>> the safepoint anyways, no real point in not stopping direct.
>> VM_ScavengeMonitors is only used to trigger a safepoint cleanup, the 
>> VM op is not needed. Arguably this thread should actually stop here, 
>> since we are about to safepoint.
>>
>> There is also a small cleanup in vmThread.cpp where an unused method 
>> is removed.
>> And the extra safepoint is removed:
>> "// We want to make sure that we get to a safepoint regularly"
>> No we don't :)
>>
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8234086
>> Change-set:
>> http://cr.openjdk.java.net/~rehn/8234086/v1/webrev/index.html
>>
>> Tested scavenge manually, passes t1-2.
>>
>> Thanks, Robbin