RFR: 8223147: JFR Backport (to jdk8u)
Andrey Petushkov
andrey at azul.com
Wed May 8 13:27:59 UTC 2019
Hi Mario,
Right, this makes sense. I'll do a cleanup for non-jfr enabled builds
Thank you,
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>
More information about the jdk8u-dev
mailing list