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

Zhengyu Gu zgu at redhat.com
Wed Nov 25 16:21:07 UTC 2020


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