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

Wed Feb 27 16:45:27 UTC 2019

Hi Mario,

First of all great thank you for the interest and such immediate involvement into this effort.
I'm sure it will help to move it faster towards having done and adopted into jdk8u mainline :)

To your particular question, what are the evidence that jfr files get compiled? It's a bit tricky thing hence the question.
There are in fact two processes which might or might not happen:
- jfr*.cpp files get compiled and linked into libjvm.so. Also JFR classes get compiled and put into jfr.jar. 
This should happen only when --enable-jfr is turned on (and yes, it's on by default)
- jfr metadata is processed and respective java and c++ classes are generated (into hotspot/linux_amd64_compiler2/generated/tools/jfr, 
hotspot/linux_amd64_compiler2/generated/jfrfiles and jdk/lib/jfr under the build directory). This in fact happens regardless of whether 
JFR is enabled or not

So if you only see the latter but don't see any jfr*.o or jfr.jar files that is normal (and that's what I observe with the code I've used to generate webrev from). 
Otherwise something is wrong. Please let me know what is the case for you

Regarding why it works that way, it's not something I've invented. It's exactly the way JFR gets built in jdk11 codebase. If it's unacceptable I'll work to change it, 
but please be aware that in order to avoid generation of jfr*.hpp files one would need to put a lot of extra #ifdefs into non-jfr specific files. Which I tried to avoid 
to keep the code as close to the original as possible (and seeing no value in doing such change, in fact).

Re default value for --enable-jfr, thank you, I've noted that. Will fix

Thanks,
Andrey

> On 27 Feb 2019, at 17:00, Mario Torre <neugens at redhat.com> wrote:
> 
> 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