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

[Mario Torre]

On Wed, 2020-05-13 at 09:56 +0800, Denghui Dong wrote:
> Gentle ping?
> 

Hi Denghui,

Sorry, the patch is really big so it's taking some time, but I asked
some help ;)

The two things that come quickly though are the commented out code
blocks and the backport bug manually created.

For the first, I think it's better to remove all those blocks of
commented code, they will just make it more difficult to apply further
fixes, also all those XXX don't really make a lot of sense to anyone
else but the person who is working on the backport or the feature in
the first place, they should not be present. If we have the need for
them, it means the backport is not complete and should be addressed,
alternatively a bug should be filed with the proper description of the
problem.

I know we let some of those slip in the JFR backport, but that wasn't
really fully intentional.

Regarding the backport bug, it will be created automatically for you
when you do commit, so you don't need to create them manually.

The patch looks good otherwise, but I need a bit more time before
completing the review.

Cheers,
Mario

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

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