[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