RFR(M): 8218049: Survivor MemoryMXBean used() size granularity is region based

[Thomas Schatzl]

Hi all,

On Wed, 2019-04-03 at 01:05 -0700, sangheon.kim at oracle.com wrote:
> Hi all,
> 
> Here's the second webrev which includes some recommendations from
> Thomas:
> 1) Fixing memory ordering issue

  just to make this clear to other reviewers: there is and have been
issues where the calculation can result in values that are not what the
GC sees due to memory ordering (i.e. negative amount of memory usage in
old gen changed to zero internally for consumption). My suggestion
adding memory barriers avoids the need for the "subtract_up_to_zero"
method (I believe ;))

It is not the goal of this change, and at this time there is no intent
to make all these values reflect the internal state atomically (i.e. is
a pre-existing problem). I have some idea how this could be done, but
it seems a lot of work; if somebody is interested I could write up
something, so contributors welcome.

Another suggestion was to make these values storing the various "used"
bytes volatile to avoid the compiler re-reading them during the
calculations.

> 2) Changing how G1SurvivorRegions collect data.
> 3) Some minor refactoring.
> 
> http://cr.openjdk.java.net/~sangheki/8218049/webrev.1
> http://cr.openjdk.java.net/~sangheki/8218049/webrev.1_inc_0/
> Testing: hs-tier 1 ~ 5, survivorAlignment/* tests

Some further minor comments:

- g1CollectedHeap.cpp:4650: please add an assert(dest.is_young(),
"...") here.

- g1EdenRegions.hpp:36: s/i.e. Updated/I.e. updated"

- g1MonitoringSupport.cpp:213: s/always doesn't/never :) (or just
remove the "always")

- g1MonitoringSupport.cpp:282: the comment is not up-to-date with
regards what is actually done (for survivor values). Survivor values
should still be consistent as they are written during a safepoint, so
the assert must still hold. Just the comment is wrong now.

Looks good to me otherwise.

Thanks,
  Thomas