RFR: 8247524: Remove unnecessary volatile qualifiers from member functions

[Kim Barrett]

> On Jun 15, 2020, at 2:59 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Kim,
> 
> On 15/06/2020 9:15 am, Kim Barrett wrote:
>> Please review this change to oopDesc and Metadata classes, removing
>> unnecessary volatile qualifiers from some member functions.
>> This change also makes virtual overrides of the relevant functions
>> explicitly virtual.
>> While there, removed unnecessary workaround in oopDesc::klass_or_null_acquire.
>> After this change,
>>   egrep -r -H --exclude-dir=.hg "\)\s*(const |)\s*volatile" src/hotspot
>> finds nothing; there seem to be no remaining volatile qualified methods.
> 
> I must admit I'm quite confused as to why anyone would ever have placed a volatile modifier on a method that returns a constant true or false ??? What could it possibly mean?

The only thing I can think of is that these were called from other member functions that
were themselves volatile qualified.  But there aren’t any of those anymore...

> On the other hand for a mutable field e.g.:
> 
> size_t size() const volatile { return _word_size; }
> 
> I can see an expectation that the volatile would force the compiler to always reload _word_size, even if the field itself was not declared volatile (which it isn't). Isn't this how we are now handling things via Atomic::load - where the field is no longer flagged volatile but the load is so that we still tell the compiler not to make assumptions about potentially caching the field?

The “preferred" way to do that today would be to make the member volatile and use Atomic
accesses (possibly relaxed if no ordering is needed).  Or just rely on the volatile qualified
member (though that’s no longer “preferred”).  (And AtomicValue<> might be used in the
future, instead of a volatile member.)

But in this case, none of that matters.  The _word_size is initialized by the constructor and
is never modified - there are no calls to set_size.  I should have removed set_size but
forgot; I’ll fix that now.  And it looks like there are a whole bunch of unused or seemingly
useless functions in Metabase.

> 
> Thanks,
> David
> 
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8247524
>> Webrev:
>> https://cr.openjdk.java.net/~kbarrett/8247524/open.00/
>> Testing:
>> mach5 tier1-3