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

[Kim Barrett]

> On Nov 20, 2019, at 4:38 AM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
> On 11/19/19 4:55 PM, Kim Barrett wrote:
>> src/hotspot/share/runtime/synchronizer.cpp
>> 1006   if (Atomic::load(&ForceMonitorScavenge) == 0 && Atomic::xchg (1, &ForceMonitorScavenge) == 0) {
>> 1007     VMThread::check_cleanup();
>> I was going to make a couple nit-pick comments here, but this seems to
>> be soon to be dead code (only reached when deprecated MonitorBound has
>> a non-default value).
>> Not sure it's worth changing this and adding check_cleanup just for
>> use here though, given this code is going away soon. Are there planned
>> future uses of check_cleanup?
> 
> No future plans, no.
> Are you suggesting doing a synchronous safepoint here instead?

I think I (and maybe David too) need to better understand what is being done here and why.

>> src/hotspot/share/runtime/vmOperations.hpp
>> Removed:
>>  156   virtual ~VM_Operation() {}
>> Simple removal may cause warnings with some flags that have been
>> discussed being enabled. This change violates the "usual rule" of
>> public and virtual or non-public for base class destructors. Of
>> course, that rule (and any warnings that might try to check for it)
>> doesn't account for non-heap-allocatable classes. And HotSpot is full
>> of violations of that rule, so we're probably a long way from being
>> able to turn on some of those kinds of warnings.
>> OTOH, leaving the destructor alone has very little cost.
> 
> Added back.

Robbin let me know offline that just adding back the virtual
destructor doesn't work.

I had forgotten that a virtual destructor doesn't work for classes
derived from StackObj, running afoul of the deleting destructor.

Since deriving from StackObj prevents the problems the various
(currently not enabled) warning options are trying to prevent, no
current harm in using the public default destructor. We'll deal with
the possible future warnings problem in the possible future.

So go ahead with your original change of using the default destructor.