Re: [8u] RFR 8225797: [backport] OldObjectSample event creates unexpected amount of checkpoint data

[Denghui Dong]

Gentle ping?


------------------------------------------------------------------
From:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>
Send Time:2020年5月6日(星期三) 17:01
To:jdk8u-dev <jdk8u-dev at openjdk.java.net>; "Jaroslav Bachorík" <jaroslav.bachorik at datadoghq.com>; Mario Torre <neugens at redhat.com>
Subject:Re: [8u] RFR 8225797: [backport] OldObjectSample event creates unexpected amount of checkpoint data

Thank you, Jaroslav!

@Mario, Could you review this backport?

Denghui Dong
------------------------------------------------------------------
From:Jaroslav Bachorík <jaroslav.bachorik at datadoghq.com>
Send Time:2020年5月6日(星期三) 15:38
To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>; jdk8u-dev <jdk8u-dev at openjdk.java.net>
Subject:Re: [8u] RFR 8225797: [backport] OldObjectSample event creates unexpected amount of checkpoint data

Hi Dennghui,

Thanks for taking care of this. The change looks good but with the
usual disclaimer "I am not a reviewer".

Cheers,

-JB-

On Wed, May 6, 2020 at 7:25 AM Denghui Dong <denghui.ddh at alibaba-inc.com> wrote:
>
> Hi Jaroslav,
>
> Sorry for the late reply due to a long holiday.
>
> > 1. http://cr.openjdk.java.net/~ddong/8225797/8u/webrev.00/src/share/vm/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp.sdiff.html
> > L113 and L116 - perhaps the commented out asserts can be removed
> > completely?
> fixed.
>
> > 2. http://cr.openjdk.java.net/~ddong/8225797/8u/webrev.00/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeManager.cpp.sdiff.html
> > L158 - `module_lock` - > `package_lock`?
> fixed.
>
> Thanks for the comments.
> Webrev is still located in http://cr.openjdk.java.net/~ddong/8225797/8u/webrev.00
>
> Cheers,
> Denghui Dong
>
> ------------------------------------------------------------------
> From:Jaroslav Bachorík <jaroslav.bachorik at datadoghq.com>
> Send Time:2020年4月29日(星期三) 18:49
> To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>
> Cc:jdk8u-dev <jdk8u-dev at openjdk.java.net>
> Subject:Re: [8u] RFR 8225797: [backport] OldObjectSample event creates unexpected amount of checkpoint data
>
> Hi Denghui,
>
> I have validated that the backport contains the changes from the
> original changeset and that the conflicts were resolved correctly.
>
> I have only two minor comments:
>
> 1. http://cr.openjdk.java.net/~ddong/8225797/8u/webrev.00/src/share/vm/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp.sdiff.html
> L113 and L116 - perhaps the commented out asserts can be removed
> completely?
> 2. http://cr.openjdk.java.net/~ddong/8225797/8u/webrev.00/src/share/vm/jfr/recorder/checkpoint/types/jfrTypeManager.cpp.sdiff.html
> L158 - `module_lock` - > `package_lock`?
>
> Cheers,
>
> -JB-
>
> On Sun, Apr 26, 2020 at 3:52 PM Denghui Dong
> <denghui.ddh at alibaba-inc.com> wrote:
> >
> > Hi team,
> > Please review the following backport.
> >
> > Original Bug: https://bugs.openjdk.java.net/browse/JDK-8225797
> > Original patch: https://hg.openjdk.java.net/jdk-updates/jdk11u/rev/2a13c6785ad2
> > Webrev: http://cr.openjdk.java.net/~ddong/8225797/8u/webrev.00/
> >
> > Some code has been commented out because those facilities(e.g. Module and Package) is not supported in 8u,
> > so the original patch can not apply cleanly. Matter of fact, the backport is mostly manual.
> > I put all reject files in http://cr.openjdk.java.net/~ddong/8225797/rej/
> >
> > All tests from jdk_jfr are passing after this patch has been applied.
> >
> > Cheers,
> > Denghui Dong