[DMARC FAILURE] [11u] [JFR] RFR 8225797: OldObjectSample event creates unexpected amount of checkpoint data

Tue Jan 7 10:21:34 UTC 2020

Hi Jaroslav,

thanks for the review. I’ve pushed JDK-8209802 which leaves us with the actual JDK-8225797. I’ll try to run it through our build/test system now. As I’ve written a while ago when I initially applied your whole queue, there were errors, even during build on some platforms/configurations. So, I’ll try once again and get back to you when I have results.

Best regards
Christoph

From: Jaroslav Bachorík <jaroslav.bachorik at datadoghq.com>
Sent: Montag, 6. Januar 2020 10:01
To: Langer, Christoph <christoph.langer at sap.com>
Cc: jdk-updates-dev <jdk-updates-dev at openjdk.java.net>
Subject: Re: [DMARC FAILURE] [11u] [JFR] RFR 8225797: OldObjectSample event creates unexpected amount of checkpoint data

Hi Christoph,

the change looks good and makes sense. Please, go ahead!

-JB-

On Sun, Jan 5, 2020 at 11:50 PM Langer, Christoph <christoph.langer at sap.com<mailto:christoph.langer at sap.com>> wrote:
Hi Jaroslav,

looking at 8209802, I’d like to make a small modification to your proposed webrev: http://cr.openjdk.java.net/~clanger/webrevs/8209802.11u/

I think, the changes done in src/hotspot/share/gc/shared/gcTrace.cpp should be guarded by #if INCLUDE_G1GC as the original changes live in a G1 specific file (src/hotspot/share/gc/g1/g1Trace.cpp). Other than that, it matches your original webrev. What do you think?

In case this gets reviewed, I’m ready to push. The patch has passed all our regression testing.

Best regards
Christoph

From: Jaroslav Bachorík <jaroslav.bachorik at datadoghq.com<mailto:jaroslav.bachorik at datadoghq.com>>
Sent: Donnerstag, 2. Januar 2020 14:37
To: Langer, Christoph <christoph.langer at sap.com<mailto:christoph.langer at sap.com>>
Cc: jdk-updates-dev <jdk-updates-dev at openjdk.java.net<mailto:jdk-updates-dev at openjdk.java.net>>
Subject: Re: [DMARC FAILURE] [11u] [JFR] RFR 8225797: OldObjectSample event creates unexpected amount of checkpoint data

Great to see this moving fast! Thanks!

On Thu, Jan 2, 2020 at 10:57 AM Langer, Christoph <christoph.langer at sap.com<mailto:christoph.langer at sap.com>> wrote:
Hi Jaroslav,

done with the next step. JDK-8214850 successfully runs through our test system. I applied it manually and came to the same result as you in your webrev (http://cr.openjdk.java.net/~jbachorik/8214850/webrev.00/) . So I'll approve and push now and will then apply and test the last prereq change JDK-8209802.

Best regards
Christoph

> -----Original Message-----
> From: Langer, Christoph
> Sent: Montag, 30. Dezember 2019 09:59
> To: Jaroslav Bachorík <jaroslav.bachorik at datadoghq.com<mailto:jaroslav.bachorik at datadoghq.com>>; jdk-updates-dev
> <jdk-updates-dev at openjdk.java.net<mailto:jdk-updates-dev at openjdk.java.net>>
> Subject: RE: [DMARC FAILURE] [11u] [JFR] RFR 8225797: OldObjectSample
> event creates unexpected amount of checkpoint data
>
> Hi Jaroslav,
>
> I just reviewed and tested the backport for JDK-8210024. It mostly applies,
> just one trivial resolve in thread.cpp is necessary. I end up with the same
> patch as in your webrev. Running it through jdk11u-dev build & testing shows
> no problems so I'll approve and push it now and continue with JDK-8214850.
>
> Best regards
> Christoph
>
>
> > -----Original Message-----
> > From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net<mailto:jdk-updates-dev-bounces at openjdk.java.net>> On
> > Behalf Of Jaroslav Bachorík
> > Sent: Freitag, 15. November 2019 16:46
> > To: jdk-updates-dev <jdk-updates-dev at openjdk.java.net<mailto:jdk-updates-dev at openjdk.java.net>>
> > Subject: [DMARC FAILURE] [11u] [JFR] RFR 8225797: OldObjectSample
> event
> > creates unexpected amount of checkpoint data
> >
> > Hi,
> >
> > I would like to get this non-trivial JFR related backport reviewed. This
> > backport is required because OldObjectSample event in the current (JDK
> 11)
> > form will cause unlimited growth of the recording thus making this event
> > unsuitable in production memory leak detection - which is its main
> purpose.
> >
> > The backport for 8225797 requires several other changes to be backported
> as
> > well. I am listing the changes and the related webrevs here in order to
> > have all the pieces in one place. The webrevs, unfortunately, are showing
> > cumulative changes, so please, disregard that information. Each webrev is
> > generated for that particular commit.
> >
> > The following tests were run successfully:
> > - jdk_tier1
> > - jdk_tier2
> > - jdk_jfr
> > - jdk_core
> >
> > ---
> >
> > Prerequisites:
> > # 8209850 : Allow NamedThreads to use GlobalCounter critical sections
> > - applied almost cleanly with only minimal changes to account for slightly
> > different code structure (patch diff -
> > http://cr.openjdk.java.net/~jbachorik/8209850/webrev.00/patch.diff)
> > JIRA .   : https://bugs.openjdk.java.net/browse/JDK-8209850
> > Webrev : http://cr.openjdk.java.net/~jbachorik/8209850/webrev.00/
> >
> > # 8209976: Improve iteration over non-JavaThreads
> > - applied almost cleanly with only minimal changes to account for slightly
> > different code structure  (patch diff -
> > http://cr.openjdk.java.net/~jbachorik/8209976/webrev.00/patch.diff)
> > - JIRA . : https://bugs.openjdk.java.net/browse/JDK-8209976
> > - Webrev : http://cr.openjdk.java.net/~jbachorik/8209976/webrev.00/
> > <http://cr.openjdk.java.net/~jbachorik/8209850/webrev.00/>
> >
> > # 8210024: JFR calls virtual is_Java_thread from ~Thread()
> > - applied almost cleanly with only minimal changes to account for slightly
> > different code structure  (patch diff -
> > http://cr.openjdk.java.net/~jbachorik/8210024/webrev.00/patch.diff)
> > - JIRA . : https://bugs.openjdk.java.net/browse/JDK-8210024
> > - Webrev : http://cr.openjdk.java.net/~jbachorik/8210024/webrev.00/
> >
> > # 8214850: Rename vm_operations.?pp files to vmOperations.?pp files
> > - applied almost cleanly with only minimal changes to account for slightly
> > different code structure  (patch diff -
> > http://cr.openjdk.java.net/~jbachorik/8214850/webrev.00/patch.diff)
> > - JIRA . : https://bugs.openjdk.java.net/browse/JDK-8214850
> > - Webrev : http://cr.openjdk.java.net/~jbachorik/8214850/webrev.00/
> >
> > # 8209802: Garbage collectors should register JFR types themselves to
> avoid
> > build errors.
> > - applied almost cleanly with only minimal changes to account for slightly
> > different code locations for g1 sources (patch diff -
> > http://cr.openjdk.java.net/~jbachorik/8209802/webrev.00/patch.diff)
> > - JIRA . : https://bugs.openjdk.java.net/browse/JDK-8209802
> > - Webrev : http://cr.openjdk.java.net/~jbachorik/8209802/webrev.00/
> >
> >
> > Backport:
> > # 8225797: OldObjectSample event creates unexpected amount of
> > checkpoint
> > data
> > - the original patch had to be accommodated to the JDK 11 status of JFR to
> > minimize the number of prerequisite backports and therefore it is slightly
> > more complex (patch diff -
> > http://cr.openjdk.java.net/~jbachorik/8225797/webrev.00/patch.diff)
> > - JIRA . : https://bugs.openjdk.java.net/browse/JDK-8225797
> > - Webrev : http://cr.openjdk.java.net/~jbachorik/8225797/webrev.00/
> >
> >
> > Thanks!
> >
> > -JB-