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

[coleen.phillimore at oracle.com]

On 8/4/20 6:56 AM, David Holmes wrote:
> Hi Markus,
>
> On 4/08/2020 7:39 pm, Markus Gronlund wrote:
>> 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 for clarifying that.

Yes, thank you Markus.
Coleen

>
> David
>
>> 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
>>>