RFR (M) 8199282: Remove ValueObj class for allocation subclassing for gc code

[Kim Barrett]

> On Mar 12, 2018, at 10:49 AM, coleen.phillimore at oracle.com wrote:
> 
> This is the last of the installments of removing ValueObj for allocation subclassing.  This removes the type from allocation.hpp. This also removes some #includes for allocation.hpp from gc files that no longer need it for VALUE_OBJ_CLASS_SPEC subclassing.  We could do an additional pass to remove more allocation.hpp and globalDefinitions.hpp from files or remove this #include on a case-by-case basis.   That is, be aware that it may no longer be needed in any header files you happen to be working on.  Thanks to Kim for pointing this out.
> 
> Tested with mach5 tier1 and 2 on linux-x64, solaris-sparcv9, macos-x64 and windows-x64.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8199282.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8199282
> 
> Thanks,
> Coleen


------------------------------------------------------------------------------
src/hotspot/share/adlc/arena.hpp
  28 // All classes in adlc may be subclassed
  29 // by one of the following allocation classes:

I think the change of "must" to "may" doesn't really describe the
intent here.  This also has terminology problems similar to those
discussed for allocation.hpp below.  Better would be to use similar
wording, or perhaps even refer to allocation.hpp.

------------------------------------------------------------------------------ 
src/hotspot/share/gc/g1/g1YoungGenSizer.hpp

Probably only needs globalDefinitions.hpp now, rather than allocation.hpp.

------------------------------------------------------------------------------ 
src/hotspot/share/gc/shared/oopStorage.inline.hpp

The include of oop.hpp should not be removed.

------------------------------------------------------------------------------ 
src/hotspot/share/memory/allocation.hpp
  40 // For convenience and documentation of intended use, classes in the virtual machine
  41 // may be subclassed by one of the following allocation classes. These provide default

[pre-existing messed up terminology]

These allocation classes don't subclass anything; the phrase
"subclassed by" is wrong.  It should be "derived from".

------------------------------------------------------------------------------
src/hotspot/share/memory/allocation.hpp
  42 // [...].  If not subclassed with the following,
  43 // the global operator new may not be used and will give a fatal error at runtime.

The phrase "If not subclassed with" 

I think better would be something like "The virtual machine must never
call one of the global allocation or deallocation functions
(operator new(size_t), operator new[](size_t), operator delete(void*), or
operator delete[](void*)).

Note that the set of functions changes with C++14, and again with
C++17.

------------------------------------------------------------------------------ 
src/hotspot/share/memory/allocation.hpp
  91 // We avoid the superclass in product mode since some C++ compilers add
  92 // a word overhead for empty super classes, also this has a vptr for print functions.

It's not just some compilers, as there are situations where no compiler
can provide the empty base optimization; see the corresponding
discussion in JDK-8173070.  How about just saying "We avoid the
superclass in product mode to save space."

------------------------------------------------------------------------------ 
src/hotspot/share/memory/memRegion.hpp
  37 // objects. These should never be allocated in heap but we do
  38 // create MemRegions (in CardTableModRefBS) in heap so operator
  39 // new and operator new [] added for this special case.

"These should never be allocated in heap", except we do.  Bleh!

And really, we only allocate MemRegion[]'s. It would probably be
better to just make the couple of places that do so instead use
NEW_C_HEAP_ARRAY and the like, rather than cluttering this class with
allocation functions.

------------------------------------------------------------------------------ 
src/hotspot/share/runtime/osThread.hpp

 56 // I'd make OSThread a ValueObj embedded in Thread to avoid an indirection, but
  57 // the assembler test in java.cpp expects that it can install the OSThread of
  58 // the main thread into its own Thread at will.

Removal of the comment doesn't seem right, unless we're really never
going to change this.  Just deleting "a ValueObj" would be sufficient
here.

Any idea what assembler test is being referred to here?  I didn't look
too hard, but I didn't find what prevents embedding OSThread into
Thread.

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