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

[coleen.phillimore at oracle.com]

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