Re: [Preliminary Review]: Proposal for back-porting JFR to OpenJDK8u

Fri Mar 1 12:38:33 UTC 2019

Hi Mario, Andrey

Very happy to see things moving forward. Some of my updates:

As Andrey and I both agreed, in order not to waste resources, Alibaba will be responsible for merging the missing features. The work is in progress, my colleague Denghui and I are working on below tasks.

- thread sampling. [code completed, under testing]
- biased locking events [code completed, under testing]
- G1 heap summary and region types  [not started yet]
- -XX:+/-EnableJFR run time flag [not started yet]

For the first three missing features, once we complete the coding and testing, we will send out webrev. For the last one, we added an 'EnableJFR' flag to enable or disable JFR functionality at runtime. Jdk11 does not have such a runtime flag. So we didn't start working right away, we want to hear the opinions of the community first.

There is one thing we think should be mentioned. Azul's patch removes the trace feature (JEP 167: Event-based JVM tracing, am I correct?) and replaces it with jfr, which is the same as jdk11. I am not sure if anyone is relying on the original trace feature. Maybe we should keep it. What do you think?

Thanks,
Guangyu
------------------------------------------------------------------
Sender:Mario Torre <neugens at redhat.com>
Sent At:2019 Feb. 28 (Thu.) 00:00
Recipient:Andrey Petushkov <andrey at azul.com>; guangyu.zhu <guangyu.zhu at aliyun.com>
Cc:jdk8u-dev <jdk8u-dev at openjdk.java.net>
Subject:[Preliminary Review]: Proposal for back-porting JFR to OpenJDK8u

On Fri, 2019-02-08 at 17:39 +0000, Andrey Petushkov wrote:
> Dear team!

Hello Andrey (and everyone else),

Some notes first :)

I gave a look at both Azul and Alibaba patches, they are obviously very
similar so I went with the latest consensus that is to integrate Azul
patch first and then add some of the missing functionality from what
Alibaba did, I also believe this makes it a tiny bit easier since it
can be considered a progressive delta, however if the opposite is
desired, please let me know.

I changed the subject line and cleaned the recipient list to make it a
bit easier to follow the discussion, I expect people interested in this
patch to be subscribed to jdk8u-dev mailing list anyway.

I will file a bug report to track the back porting effort, I think we
will need to include the CSR at some point in the process, however I'll
leave this to Andrew Haley as project maintainer, I think for the time
being we can experiment on the incubator branch.

Also, we will need to keep this branch updated with latest changes from
the primary repository, it is appreciated if other committers can
participate in this task.

Finally, I take this opportunity to thank you and Guangyu and everyone
else involved for those patches and for your patience as well as
understanding regarding the process.

Ok, now on with the interesting bits ;)

I applied the patch from http://cr.openjdk.java.net/~apetushkov/jfr8/
as described in you email below and compiled everything, --enable-jfr is enabled by default I think this should be disabled by default instead, however, when compiling even if I pass --disable-jfr I still see that the jfr files gets compiled and everything, so the option doesn't appear to do anything. What is the proper way to disable it?

Cheers,
Mario

> It happened that in the meanwhile Azul was also working on preparing
> the backport of JFR to jdk8 codebase. We as well have finished the
> implementation recently and would like to contribute it to the
> community. Our approach and platform coverage is a bit different so
> patches are definitely not the same. One might want to compare and
> merge somehow to get most of both. We are performing difference
> analysis but not yet finished. Will update once we have full
> information.
> The following is different to your approach:
> - We did not add EnableJFR flag, JFR could be used in the same way as
> in jdk11
> - we have added —enable-jfr parameter to configure script to control
> inclusion of JFR into binary
> - -XX:+/-LogJFR VM parameter to control JFR logging (analogous to
> -Xlog:jfr*=info), with -XX:+Verbose acting like -Xlog:jfr*=trace
> - we have removed the “trace*” build files while adding “jfr*” files.
> We believe such approach makes it easier to backport further jfr
> fixes from jdk11+
> - we have the following platforms supported: Linux x86, ppc, arm, all
> in both 32-bit and 64-bit variants. Also Mac and Windows x86 32/64
> are supported (Solaris on x86 and sparc have some bugs which are not
> resolved yet)
> We as well have JFR test suite passed with some limitations coming
> from missing VM features (we did not add some missing WhiteBox APIs
> so test coverage is a bit smaller than yours)
> There might be as well difference in data being supported, e.g. Azul
> implementation does not support G1 memory types, while Alibaba seem
> to have added missing G1 functionality to get this data
> 
> The webrev is at
> http://cr.openjdk.java.net/~apetushkov/jfr8/
> Baseline is 8u202.
> The changes to aarch64- and aarch32-port projects are delta on top of
> generic hotspot patch.
> Shenandoah GC is not integrated with JFR. BTW, related question to
> RedHat, I see that aarch64-port/jdk8u repos are stale for ~5 months,
> and latest update level is u181. Does that mean that OpenJDK aarch64-
> jdk8u mainline is considered to be hosted in 
> http://hg.openjdk.java.net/aarch64-port/jdk8u-shenandoah/ repos
> (where 8u192 is propagated for some time already)? Just in case I’ve
> posted integration patches for both repos
> 
> Regards,
> Andrey
> 
> 
> Begin forwarded message:
> 
> From: "guangyu.zhu" <guangyu.zhu at aliyun.com<mailto:
> guangyu.zhu at aliyun.com>>
> Subject: Re: Proposal for back-porting JFR to OpenJDK8u
> Date: January 29, 2019 at 3:41:35 AM PST
> To: "Andrew Haley" <aph at redhat.com<mailto:aph at redhat.com>>, "Mario
> Torre" <neugens.limasoftware at gmail.com<mailto:
> neugens.limasoftware at gmail.com>>, "jdk8u-dev" <
> jdk8u-dev-bounces at openjdk.java.net<mailto:
> jdk8u-dev-bounces at openjdk.java.net>>
> Cc: yumin qi <yumin.qi at gmail.com<mailto:yumin.qi at gmail.com>>,
> kingsum.chow <kingsum.chow at gmail.com<mailto:kingsum.chow at gmail.com>>,
> jdk8u-dev <jdk8u-dev at openjdk.java.net<mailto:
> jdk8u-dev at openjdk.java.net>>, denghui.ddh <denghui.ddh at antfin.com<mai
> lto:denghui.ddh at antfin.com>>
> Reply-To: guangyu.zhu <guangyu.zhu at aliyun.com<mailto:
> guangyu.zhu at aliyun.com>>
> 
> Hi there,
> 
> JFR backport patch has been uploaded to cr.openjdk. Please have a
> review for the patch.
> 
> Webrev:
> http://cr.openjdk.java.net/~luchsh/hs_jfr_cr/
> http://cr.openjdk.java.net/~luchsh/jdk_jfr_cr/
> 
> The original patch comes from webrev: 
> http://cr.openjdk.java.net/~mgronlun/JEP328_FlightRecorder/Preview/webrev/index.html
>  . We ported it to jdk8u192-b26 and passed most of the jfr jtreg
> tests on Linux/x86-64 platform. Some features related to Module, AOT
> and log level are removed because jdk8 does not support them.
> 
> We have added a new option ‘EnableJFR’ to enable or disable the JFR
> feature. It’s disabled by default. To enable it, you can start java
> with ‘-XX:+EnableJFR’.  For example:
> java -XX:+EnableJFR
> -XX:StartFlightRecording=duration=1m,filename=rec.jfr MyApp
> 
> When running the jtreg test, please add the extra option '-vmoption:-
> XX:+EnableJFR'. Otherwise the test case will not execute correctly.
> Here is an example of running jfr jtreg test:
> make test JTREG_TEST_EXTRA_OPTIONS=-vmoption:-XX:+EnableJFR
> TEST=jdk_jfr
> 
> There is one more thing worth noting, please use jmc version 7.0 or
> later to open the jfr record file.
> 
> Your suggestions and comments are welcome.
> 
> Thanks,
> Guangyu Zhu
> ------------------------------------------------------------------
> Sender:李三红(三红) <sanhong.lsh at alibaba-inc.com<mailto:
> sanhong.lsh at alibaba-inc.com>>
> Sent At:2018 Dec. 17 (Mon.) 21:21
> Recipient:Andrew Haley <aph at redhat.com<mailto:aph at redhat.com>>; Mario
> Torre <neugens.limasoftware at gmail.com<mailto:
> neugens.limasoftware at gmail.com>>
> Cc:jdk8u-dev <jdk8u-dev at openjdk.java.net<mailto:
> jdk8u-dev at openjdk.java.net>>
> Subject:Re: Proposal for back-porting JFR to OpenJDK8u
> 
> Thanks for comments, we are preparing our internal patches for
> webrev.
> In the meantime, Martijn/ Andrew mentioned we can use AdoptOpenJDK to
> produce technical preview build.
> Would like to know this... can Martijn point us some guide somewhere?
> 
> Thanks!
> Sanhong
> -----邮件原件-----
> 发件人: Andrew Haley [mailto:aph at redhat.com]
> 发送时间: 2018年12月13日 3:03
> 收件人: Mario Torre <neugens.limasoftware at gmail.com<mailto:
> neugens.limasoftware at gmail.com>>; 李三红(三红) <
> sanhong.lsh at alibaba-inc.com<mailto:sanhong.lsh at alibaba-inc.com>>
> 抄送: Volker Simonis <volker.simonis at gmail.com<mailto:
> volker.simonis at gmail.com>>; jdk8u-dev at openjdk.java.net<mailto:
> jdk8u-dev at openjdk.java.net>
> 主题: Re: Proposal for back-porting JFR to OpenJDK8u
> 
> On 12/12/18 1:44 PM, Mario Torre wrote:
> I think the first thing to do would be to post the patch for review,
> a
> shared repository would make the review process a bit more complex I
> think, and only makes sense if there is a need to work further on the
> patch collectively.
> 
> Yes, exactly. That's the normal process, and it's the best place to
> start.
> 
> --
> Andrew Haley
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
> 
> 
> 
> 
> 
-- 
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898