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

[Markus Gronlund]

Hello,

About _last_sweep:

The value, representing the last time the GC passed over the sample instances, is only treated as a hint, used as input to construct a filter when serializing object samples as events along with their associated chains. The filter excludes samples that are short-lived, i.e. having timestamps after the _last_sweep value. _last_sweep is only read by a single thread which at that moment have exclusive access to the sampler instance. It is ok for it to read whatever value leaked to it, in fact it can be advantageous to see an older value as it makes the filter more effective in reducing noise. Therefore, I think it is ok to leave an undecorated access to this variable. Also, there is no ordering constraint, the value is just reset when constructing a new instance, and a race is ok here too.

Thanks
Markus

-----Original Message-----
From: David Holmes 
Sent: den 4 augusti 2020 01:26
To: Coleen Phillimore <coleen.phillimore at oracle.com>; Kim Barrett <kim.barrett at oracle.com>
Cc: hotspot-dev developers <hotspot-dev at openjdk.java.net>; hotspot-jfr-dev at openjdk.java.net
Subject: Re: RFR (M) 8235573: Move JFR ObjectSample oop into OopStorage

Hi Coleen,

On 3/08/2020 11:14 pm, coleen.phillimore at oracle.com wrote:
<snip>
> 
> incremental webrev at
> http://cr.openjdk.java.net/~coleenp/2020/8235573.02.incr/webrev
> full webrev at 
> http://cr.openjdk.java.net/~coleenp/2020/8235573.03/webrev

src/hotspot/share/gc/shared/weakProcessorPhases.hpp

Have you built this with JVMTI disabled? I'm not sure if an empty enum declaration might produce a warning?

---

src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp

Are there any ordering constraints on these store pairs:

  68     Atomic::store(&_dead_samples, true);
  69     _last_sweep = JfrTicks::now();

  93   _last_sweep = JfrTicks::now();
  94   Atomic::store(&_dead_samples, false);

?

---

Otherwise this all seems good.

Thanks,
David
-----


> And retested.
> Thanks,
> Coleen
>