Re: Proposal for back-porting JFR to OpenJDK8u

[guangyu.zhu]

by the way, what platform are you compiling on?
------------------------------------------------------------------
Sender:guangyu.zhu <guangyu.zhu at aliyun.com>
Sent At:2019 Jan. 31 (Thu.) 21:22
Recipient:Ao Qi <aoqi at loongson.cn>
Cc:jdk8u-dev <jdk8u-dev-bounces at openjdk.java.net>; yumin qi <yumin.qi at gmail.com>; kingsum.chow <kingsum.chow at gmail.com>; jdk8u-dev <jdk8u-dev at openjdk.java.net>; denghui.ddh <denghui.ddh at antfin.com>
Subject:Re: Proposal for back-porting JFR to OpenJDK8u

Hi Ao Qi,

Thanks for your quick test, I will fix the trailing whitespaces in the next webrev update. The missing functions you listed will inevitably require the help of the community. Amazon colleagues have mentioned that they can help, hope they are still available.

Thanks,
Guangyu
------------------------------------------------------------------
Sender:Ao Qi <aoqi at loongson.cn>
Sent At:2019 Jan. 31 (Thu.) 20:14
Recipient:guangyu.zhu <guangyu.zhu at aliyun.com>
Cc:Andrew Hughes <gnu.andrew at redhat.com>; Mario Torre <neugens at redhat.com>; jdk8u-dev <jdk8u-dev-bounces at openjdk.java.net>; yumin qi <yumin.qi at gmail.com>; jdk8u-dev <jdk8u-dev at openjdk.java.net>; kingsum.chow <kingsum.chow at gmail.com>; denghui.ddh <denghui.ddh at antfin.com>
Subject:Re: Proposal for back-porting JFR to OpenJDK8u

Hi Guangyu,

I am not a jfr expert, just tried a commit and some simple build
tests. Here are some results for you:

hotspot/src/share/vm/jfr/utilities/jfrJavaLog.cpp:42: Trailing whitespace
jdk/test/jdk/jfr/event/os/TestCPUInformation.java:57: Trailing whitespace
jdk/test/jdk/jfr/jvm/TestPid.java:48: Trailing whitespace

pd_get_top_frame_for_profiling and class VM_Version_Ext are missing
and break the buildability on some non-(linux/x86-64) platforms.

Definitions of OrderAccess::load_acquire(const volatile bool*  p),
OrderAccess::load_acquire(const volatile julong*  p),
OrderAccess::load_ptr_acquire(const volatile uintptr_t* p) and
Atomic::store    (julong   store_value, julong*   dest) seem missing
on non-(linux/x86-64) platforms.

"#ifdef X86" in jfrTraceTime.cpp and os_perf_linux.cpp may should be
"#if defined(X86) && !defined(ZERO)". I believe other CPUs should be
the same.

Cheers,
Ao Qi

On Thu, Jan 31, 2019 at 12:31 PM guangyu.zhu <guangyu.zhu at aliyun.com> wrote:
>
> The risk lies in other non-linux/x86-64 platforms. We have never built on them.
>
> Thanks,
> Guangyu
> ------------------------------------------------------------------
> Sender:Hohensee, Paul <hohensee at amazon.com>
> Sent At:2019 Jan. 31 (Thu.) 08:45
> Recipient:Andrew Hughes <gnu.andrew at redhat.com>; Mario Torre <neugens at redhat.com>
> Cc:jdk8u-dev <jdk8u-dev-bounces at openjdk.java.net>; yumin qi <yumin.qi at gmail.com>; jdk8u-dev <jdk8u-dev at openjdk.java.net>; kingsum.chow <kingsum.chow at gmail.com>; denghui.ddh <denghui.ddh at antfin.com>
> Subject:Re: Proposal for back-porting JFR to OpenJDK8u
>
> The backport is on the EnableJFR switch is true, but it's default false in the patch, so JFR is default off.
>
> Paul
>
>  On 1/30/19, 6:49 AM, "jdk8u-dev on behalf of Andrew Hughes" <jdk8u-dev-bounces at openjdk.java.net on behalf of gnu.andrew at redhat.com> wrote:
>
>     On Wed, 30 Jan 2019 at 09:15, Mario Torre <neugens at redhat.com> wrote:
>     >
>     > Yeah, however I think we may need to be conservative with this, since the
>     > stability of the platform has precedence over new features, so we need to
>     > find some way to have a staging environment for those changes.
>     >
>     > Cheers,
>     > Mario
>     >
>     > On Wed, Jan 30, 2019 at 9:37 AM guangyu.zhu <guangyu.zhu at aliyun.com> wrote:
>     >
>     > > Good suggestion! Paul
>     > >
>     > > Thanks,
>     > > Guangyu
>     > >
>
>     I like Paul's suggestion. It's easier to have the core work in there
>     and easily available
>     for everyone to work on, than to try and create one perfect patch.
>     It's also then easier
>     to track what follow-on bugs have been fixed in 8u.
>
>     Does the initial patch allow this to be disabled? The risk is
>     significantly reduced if it
>     is still possible to build as now with it applied.
>
>     Thanks,
>     --
>     Andrew :)
>
>     Senior Free Java Software Engineer
>     Red Hat, Inc. (http://www.redhat.com)
>
>     Web Site: http://fuseyism.com
>     Twitter: https://twitter.com/gnu_andrew_java
>     PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
>     Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
>
>
>