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

[Mario Torre]

On Fri, Mar 1, 2019 at 1:38 PM guangyu.zhu <guangyu.zhu at aliyun.com> wrote:
>
> Hi Mario, Andrey
>
> Very happy to see things moving forward. Some of my updates:

Thanks for your work and help!

> 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.

I don't think we should diverge in any way from the other versions
unless necessary. For 8u-dev I would like to follow what Oracle did on
their VM, with the usual pack of -XX:+UnlockCommercialFeatures
-XX:+FlightRecorder -XX:StartFlightRecording=xxx options.

I know -XX:+UnlockCommercialFeatures isn't necessary in our case, but
it should be there for compatibility. I would be ok if the option
didn't do anything but was just there as to not crash the VM at
startup when someone was migrating from one downstream version to the
other. Of course, it can be said that -XX:+UnlockCommercialFeatures in
OpenJDK currently doesn't exist and is deprecated in later versions
anyway, so I defer to Andrew Haley regarding this specific point if he
has a strong opinion of not introducing the option at all.

Regarding +EnableJFR as a runtime flag, my take is that
-XX:+FlightRecorder -XX:StartFlightRecording are already doing the
same.

> 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?

I'm going through the patch right now, but yes, from what I see the
trace is removed. I had too a concern about this and was about to send
a note. I'm not quite sure what to do, because Trace has been removed
in 11 as far as I know, but removing mid stream in 8 may be a more
interesting issue, however this isn't a user facing API, it was always
meant to be internal to the JVM, so I don't quite know if there's
really a reason we shouldn't change it. This is one question for the
CSR group I think.

Cheers,
Mario
-- 
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898