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

[Zhengyu Gu]

Hi all,

Updated according to your comments:
http://cr.openjdk.java.net/~zgu/JDK-8238633/webrev.02/

I modified 
vmTestbase/nsk/jvmti/unit/heap/HeapWalkTests/TestDescription.java test 
[1] to walk 300K objects.

Without patch:
Time: 987431 nsecs
Time: 1135390 nsecs
Time: 1142519 nsecs
Time: 962816 nsecs
Time: 1015958 nsecs

Avg: 1048822 nsecs

With patch:
1105015 nsecs
1142425 nsecs
968057 nsecs
1383838 nsecs
1079885 nsecs

Avg: 1135844 nsecs

So, it shows about 8% performance hit.

Thanks,

-Zhengyu

[1] http://cr.openjdk.java.net/~zgu/JDK-8238633/test/webrev.00/





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