RFR: 8248216: JFR: Unify handling of all OopStorage instances in LeakProfiler root processing

[Erik Österlund]

Hi Coleen,

Thanks for the review.

/Erik

On 2020-06-25 15:19, coleen.phillimore at oracle.com wrote:
>
> This patch looks good to me.
> It also shows that I missed one of the places in my jvmti OopStorage 
> patch.
>
> Coleen
>
> On 6/24/20 9:29 AM, Erik Österlund wrote:
>> Hi Stefan,
>>
>> Thanks for the review.
>>
>> Updated to use your new API:
>> http://cr.openjdk.java.net/~eosterlund/8248216/webrev.01/
>>
>> Also added Markus to the conversation.
>>
>> Thanks,
>> /Erik
>>
>> On 2020-06-24 14:36, Stefan Karlsson wrote:
>>> Hi Erik,
>>>
>>> On 2020-06-24 11:13, Erik Österlund wrote:
>>>> Hi,
>>>>
>>>> We want it to be as convenient as possible for the runtime to add 
>>>> new OopStorage instances. Therefore, rather than picking specific 
>>>> OopStorages during LeakProfiler root processing, the 
>>>> OopStorageSet::strong_iterator() should be used instead. That way, 
>>>> once a new OopStorage is added to the OopStorageSet, it is 
>>>> automatically plugged in to the JFR LeakProfiler.
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8248216
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~eosterlund/8248216/webrev.00/
>>>
>>> https://cr.openjdk.java.net/~eosterlund/8248216/webrev.00/src/hotspot/share/jfr/leakprofiler/chains/rootSetClosure.cpp.udiff.html 
>>>
>>>
>>> I've now pushed a change so that you can simplify:
>>>
>>> + for (OopStorageSet::Iterator it = 
>>> OopStorageSet::strong_iterator(); !it.is_end(); ++it) {
>>> + it->oops_do(this);
>>> + } to: OopStorageSet::strong_oops_do(this); The rest looks fine, 
>>> but should maybe be reviewed on hotspot-jfr-dev. Thanks, StefanK
>>>
>>>>
>>>> Thanks,
>>>> /Erik
>>>
>>
>