RFR (M) 8235573: Move JFR ObjectSample oop into OopStorage

[Kim Barrett]

> On Jul 31, 2020, at 11:43 AM, coleen.phillimore at oracle.com wrote:
> 
> 
> Markus contributed a better incremental patch on top of this, please review tested 02 patch instead.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/2020/8235573.02/webrev
> 
> Thanks,
> Coleen
> 
> On 7/31/20 10:16 AM, coleen.phillimore at oracle.com wrote:
>> Summary: Add a weak OopStorage for JFR and move the oops there. Remove GC code to process oops.
>> 
>> Tested with tier1,5, and 6.  Built with shenandoah.
>> 
>> open webrev at http://cr.openjdk.java.net/~coleenp/2020/8235573.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8235573
>> 
>> Thanks,
>> Coleen

One less serial weak processing phase!  Yay!

------------------------------------------------------------------------------
src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp 
  66     if (!_dead_samples) {
  67       _dead_samples = true;
  68     } else {
  69       // Need exclusive access to avoid a race that could loose a
  70       // notification about new dead samples. Previously, the notification
  71       // happened during a safepoint, i.e. as part of weak_oops_do(),
  72       // but with the change to OopStorage, it can happen concurrently
  73       // with threads trying to reset the flag.
  74       acquire();
  75       _dead_samples = true;
  76       release();
  77     }

_dead_samples is potentially accessed by multiple threads, including
writers.  That suggests it needs to be an "atomic" variable.

Blocking on a potentially long-duration (spin!) lock in this gc-callback
might be a problem.  The approach taken by the string table might be better.
There is a similar race there.  The approach taken there is to just lose the
race and let the next GC trigger cleanup.  But StringTable also uses another
thread (the ServiceThread) to perform the cleanup, rather than piggy-backing
cleanup on the add() operation, which might not occur at an appropriately
timed point.

I think a structure similar to that used by the StringTable would work well
here too.

A different approach would be to have the gc-callback unconditionally set
_dead_samples if any are reported, and have scavenge clear _dead_samples
before doing its cleanup.  But that can result in back-to-back cleanups
(by successive adds) where the second might not be needed.  The StringTable
approach prefers a possibly delayed cleanup vs unnecessary cleanups.  And
it seems a delayed cleanup is acceptable here, since it won't even be
considered until an add operation following _dead_samples being set.

------------------------------------------------------------------------------
src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp 
 100   _size(size) {
 101     _last_sweep = JfrTicks::now();
 102     _dead_samples = false;
 103 }

Assignments in the body are mis-indented.  But why were they changed from
mem-initializers to assignments?

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

Including "Weak" in the oopstorage's name would be consistent with the
others, and makes things a little more obvious in GC logs.