[sh/jdk8u] RFR 8221507: Implement JFR Events for Shenandoah

Roman Kennke rkennke at redhat.com
Wed Nov 25 18:05:53 UTC 2020


Looks good to me!

Thanks,
Roman

> Hi Roman,
> 
> On 11/24/20 10:44 AM, Roman Kennke wrote:
>> Hi Zhengyu,
>>
>> Looks good so far. A few things:
>>
>> - Prepend commit message with [backport]
>> - Do not entirely remove INCLUDE_SHENANDOAHGC, replace it with 
>> INCLUDE_ALL_GCS. Do this for the #include 
>> "gc_implementation/shenandoah/shenandoahJfrSupport.hpp" too.
>> - In src/share/vm/jfr/periodic/jfrPeriodic.cpp you are removing a 
>> newline between the includes and the first block of code. Don't do that.
> 
> Thanks for reviewing.
> 
> Updated: 
> http://cr.openjdk.java.net/~zgu/shenandoah/JDK-8221507-8u/hotspot/webrev.01/ 
> 
> 
> Thanks,
> 
> -Zhengyu
> 
>>
>> Thanks for backporting this!
>>
>> Please also take care of follow-up issues JDK-8224573 and JDK-8224529, 
>> and push them together, ok?
>>
>> Thanks, Roman
>>
>>
>> On 24/11/2020 16:31, Zhengyu Gu wrote:
>>> Hi,
>>>
>>> Please review this 8u backport of JFR events for Shenandoah.
>>>
>>> The original patch does not apply cleanly.
>>>
>>> 1) The patch has to be split into hotspot and jdk parts.
>>> 2) Adjusted @library and removed @requires vm.hasJFR tag(not 
>>> supported in 8u) for tests
>>> 3) Removed INCLUDE_SHENANDOAHGC, sh/8u always builds with Shenandoah on.
>>> 4) Adjusted imports
>>> 5) ShenandoahHeapRegion::region_number() -> 
>>> ShenandoahHeapRegion::index()
>>> 6) Manually fixed some merge conflicts
>>>
>>>
>>> The original webrev: http://hg.openjdk.java.net/jdk/jdk/rev/43340a79840d
>>>
>>> sh/8u webrev:
>>>    hotspot: 
>>> http://cr.openjdk.java.net/~zgu/shenandoah/JDK-8221507-8u/hotspot/webrev.00/ 
>>>
>>>    jdk: 
>>> http://cr.openjdk.java.net/~zgu/shenandoah/JDK-8221507-8u/jdk/webrev.00/
>>>
>>> Test:
>>>    Passed new tests in patch on Linux x86_64
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>
>>
> 



More information about the shenandoah-dev mailing list