[DMARC FAILURE] [11u] [JFR] RFR 8225797: OldObjectSample event creates unexpected amount of checkpoint data
Langer, Christoph
christoph.langer at sap.com
Fri Jan 24 12:51:21 UTC 2020
Hi Jaroslav, all,
I’m done with testing your backport patch for JDK-8225797 now.
So, this was your original proposal:
JBS: https://bugs.openjdk.java.net/browse/JDK-8225797
Original Change: https://hg.openjdk.java.net/jdk/jdk/rev/caa25ab47aca
Initial backport Webrev: http://cr.openjdk.java.net/~jbachorik/8225797/webrev.00/
First of all, to make the debug build work, I had to do a very few modifications (incremental diff to your original backport): http://cr.openjdk.java.net/~clanger/webrevs/8225797.fix-debug-incremental.11u/
It was mainly about the ClassLoaderDataGraph_lock and cld->is_unsafe_anonymous() which don’t exist in jdk11 but were used in assertions. So this would only be noticed in debug builds.
So this is the final webrev with the changes worked in: http://cr.openjdk.java.net/~clanger/webrevs/8225797.11u/
Please review.
When pushing this, we also need to push the fix for https://bugs.openjdk.java.net/browse/JDK-8231025 (Incorrect method tag offset for big endian platform), otherwise we’ll see crashes and test failures in big endian platforms like sun_64 or linuxppc64 and AIX. However, this change applies with minor fuzz, so it should just be tagged with a jdk11u-fix-request label.
I also suggest to backport JDK-8231081 (TestMetadataRetention fails due to missing symbol id) immediately after 8225797 but I’ll send out a separate RFR for this.
Best regards
Christoph
From: Jaroslav Bachorík <jaroslav.bachorik at datadoghq.com>
Sent: Dienstag, 7. Januar 2020 13:26
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
Great, thanks for the update. Don't hesitate to contact me if there are any failures. Getting this in is my top priority.
-JB-
On Tue, Jan 7, 2020 at 11:21 AM Langer, Christoph <christoph.langer at sap.com<mailto:christoph.langer at sap.com>> wrote:
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<mailto:jaroslav.bachorik at datadoghq.com>>
Sent: Montag, 6. Januar 2020 10:01
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
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-
More information about the jdk-updates-dev
mailing list