RFR: 8223147: JFR Backport (to jdk8u)

[Andrey Petushkov]

Hi Mario,

Apparently the situation is not that bad, just a few things went wrong on our sides.
My fault I did not switch --enable-jfr to false. Fixed now
And you seem did not make clean after changing configure options :) With proper clean build the only wrong behavior I get from
your list is the presence of jfc/template files in the distribution. I've fixed that as well. I don't see any other JFR artefacts in the images
directory when building with JFR disabled (there are few intermediate files generated but these do not get into final image)
Your test passes
The webrevs are updated, please could you check again
Sorry, I did not adopt your test as a jtreg yet

Thanks a lot,
Andrey

> On 8 May 2019, at 15:24, Mario Torre <neugens at redhat.com> wrote:
> 
> Hi Andrey,
> 
> There are a few things I wanted to point out about this patch.
> 
> Overall, it seems good to me, of course not being different than what
> we already pre-reviewed. There are a couple of points however that we
> need to address, I think they should be addressed before the patch is
> in, but I'm open for discussions.
> 
> By default, when I compile without jfr (without --enable-jfr) there
> should be no reference to JFR in the resulting image, but this is not
> the case.
> 
> For example, in the resulting images/j2sdk-image/jre/lib library I can
> still see jfr.jar and a jfr directory with the jfr templates next to
> the rest of the libraries. I think this should be removed (a step at
> the end of the compilation process will do I think, if it's too
> cumbersome to not generate those in the first place).
> 
> What's more is that the presence of JFR sneaks into the JDK at
> runtime. For example, the attached applications when run on a default
> build of OpenJDK 8 produces the following exception:
> 
> Exception in thread "main" javax.management.RuntimeMBeanException:
> java.lang.IllegalArgumentException: VM option "FlightRecorder" does
> not exist
>    at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.rethrow(DefaultMBeanServerInterceptor.java:839)
>    at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.rethrowMaybeMBeanException(DefaultMBeanServerInterceptor.java:852)
>    at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.invoke(DefaultMBeanServerInterceptor.java:821)
>    at com.sun.jmx.mbeanserver.JmxMBeanServer.invoke(JmxMBeanServer.java:801)
>    at CheckJFR.main(CheckJFR.java:15)
> Caused by: java.lang.IllegalArgumentException: VM option
> "FlightRecorder" does not exist
>    at sun.management.HotSpotDiagnostic.getVMOption(HotSpotDiagnostic.java:83)
>    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>    at java.lang.reflect.Method.invoke(Method.java:498)
>    at sun.reflect.misc.Trampoline.invoke(MethodUtil.java:71)
>    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>    at java.lang.reflect.Method.invoke(Method.java:498)
>    at sun.reflect.misc.MethodUtil.invoke(MethodUtil.java:275)
>    at com.sun.jmx.mbeanserver.ConvertingMethod.invokeWithOpenReturn(ConvertingMethod.java:193)
>    at com.sun.jmx.mbeanserver.ConvertingMethod.invokeWithOpenReturn(ConvertingMethod.java:175)
>    at com.sun.jmx.mbeanserver.MXBeanIntrospector.invokeM2(MXBeanIntrospector.java:117)
>    at com.sun.jmx.mbeanserver.MXBeanIntrospector.invokeM2(MXBeanIntrospector.java:54)
>    at com.sun.jmx.mbeanserver.MBeanIntrospector.invokeM(MBeanIntrospector.java:237)
>    at com.sun.jmx.mbeanserver.PerInterface.invoke(PerInterface.java:138)
>    at com.sun.jmx.mbeanserver.MBeanSupport.invoke(MBeanSupport.java:252)
>    at javax.management.StandardMBean.invoke(StandardMBean.java:405)
>    at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.invoke(DefaultMBeanServerInterceptor.java:819)
>    ... 2 more
> 
> When running against the jdk8u-jfr-incubator build instead produces
> "false". If I run with the -XX:+FlightRecorder I even get a "true"
> although I then can't request a flight recorder via JMC or the mbean
> interface, while a standard build would give me "Unrecognized VM
> option 'FlightRecorder'" in this case.
> 
> I think we should fix this before merging (and please, do feel free to
> add this small snippet of code as a test case in the patch).
> 
> I'll try to give more useful feedback as I keep on testing.
> 
> Cheers,
> Mario
> 
> 
> 
> On Tue, May 7, 2019 at 3:52 PM Mario Torre <neugens at redhat.com> wrote:
>> 
>> On Tue, May 7, 2019 at 1:20 PM Andrey Petushkov <andrey at azul.com> wrote:
>>> 
>>> Hi All,
>> 
>> Hi Andrey, thanks for the patch!
>> 
>>> RFR for initial JFR backport to current state of jdk8u-jfr-incubator is here http://cr.openjdk.java.net/~apetushkov/8223147/
>>> Please could you review and hopefully approve
>> 
>> I'm going through it now, I'll get back to you as soon as possible!
>> 
>>> The changes are the same as in initial review request [1] except two fixes were added for the bugs found after integration
>>> of Alibaba's implementation of thread sampling (note that according to the plan Alibaba's patches are not included into this RFR)
>>> The webrev includes bug ids for all changes backported under it, however these are not separated on per-file basis
>> 
>> This is perfect, so we have proper tracking of the patches.
>> 
>>> One process question (thus Andrew in To), the new webrev currently only includes the code which applies to jdk8u-jfr-incubator
>>> repo, but what about aarch64 and aarch32 ports which are not available there? I don't see a reason to prevent them from
>>> having JFR and there is code ready for that ([1]). How could we proceed with integration of it?
>> 
>> You are right, it didn't occur to me that this code was not part of
>> the repository, but this is because the staging repo is forked from
>> jdk8u-dev which doesn't contain those ports.
>> 
>> I think we should indeed backport the other changes, perhaps once
>> everything is in mainline we can push into the shenandoah/arm/etc...
>> repositories individual patches?
>> 
>> 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
> 
> 
> 
> --
> Mario Torre
> Associate Manager, Software Engineering
> Red Hat GmbH <https://www.redhat.com>
> 9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898
> <CheckJFR.java>