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

[coleen.phillimore at oracle.com]

Adding serviceability-dev back.
Coleen

On 2/21/20 7:59 AM, coleen.phillimore at oracle.com wrote:
>
> 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
>>>>>
>>>>>
>>>>
>>>
>>
>