[15] RFR 8238633: JVMTI heap walk should consult GC for marking oops

[coleen.phillimore at oracle.com]

Hi, I had a quick look at this, minus the shenandoah code.

http://cr.openjdk.java.net/~zgu/JDK-8238633/webrev.01/src/hotspot/share/gc/shared/objectMarker.hpp.html

I think this file could have forward declarations of GrowableArray and I 
didn't see a need for the markWord.hpp include.

This change on the whole looks good to me.

Coleen

On 2/21/20 5:23 AM, Stefan Karlsson wrote:
> Hi Zhengyu,
>
> On 2020-02-17 15:51, Zhengyu Gu wrote:
>> Hi Stefan,
>>
>> Thanks for the review and suggestions, updated accordingly:
>>
>> http://cr.openjdk.java.net/~zgu/JDK-8238633/webrev.01/
>
> Thanks for moving the code. I think this looks good.
>
> If you're up for it, I have a couple of style change suggestions:
>
> 1) ObjectMarker uses two verbs to describe the same thing: "mark" and 
> "visit". I propose that we only use "mark" in ObjectMarker and leave 
> the usage of "visited" to the Jvmti code.
>
> 2) Some updates to odd whitespaces
>
> 3) Using forward declarations in Shenandoah code.
>
> I've bundled those changes into webrevs:
>
> https://cr.openjdk.java.net/~stefank/8238633/webrev.01.delta
> https://cr.openjdk.java.net/~stefank/8238633/webrev.01
>
> Regarding performance testing, the HeapWalkTests you used seems to use 
> a very small heap. I think it would be good to redo the measurements 
> on a larger heap. Could you take the HeapWalkTest and add a few GBs of 
> small, linked objects?
>
> Thank,
> StefanK
>>
>>>
>>> ---
>>> Previously, the calls to 'mark' and 'visited' were inlineable, but 
>>> now every GC has to take a virtual call when marking the objects. My 
>>> guess is that this code is slow anyway, and that it doesn't matter 
>>> too much, but did you measure the effect of that change with, for 
>>> example, G1?
>>>
>> I did rough measurement, timing 
>> vmTestbase/nsk/jvmti/unit/heap/HeapWalkTests/TestDescription.java test.
>>
>> If you know any tests/benchmarks I should measure, please let me know.
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>
>>> Thanks,
>>> StefanK
>>>
>>>> Test:
>>>>    hotspot_gc
>>>>    vmTestbase_nsk_jdi
>>>>    vmTestbase_nsk_jvmti
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>
>>>>
>>>
>>
>