RFR (11u, S): backport JDK-8247615 Initialize the bytes left for the heap sampler

Thu Aug 27 16:21:04 UTC 2020

Hi JC,

I've approved the backport now.

Cheers
Christoph

PS: Your mails to jdk-updates-dev still don't make it into my inbox, while Martin's mails now arrive. Maybe you should sync up with him about his mail settings ��

> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> On
> Behalf Of Volker Simonis
> Sent: Donnerstag, 27. August 2020 16:04
> To: Jean Christophe Beyler <jcbeyler at google.com>
> Cc: jdk-updates-dev <jdk-updates-dev at openjdk.java.net>
> Subject: Re: RFR (11u, S): backport JDK-8247615 Initialize the bytes left for
> the heap sampler
> 
> Hi Jc,
> 
> you need to record the review thread in the JBS issue, otherwise the
> 11u maintainers have no chance to know when a change which requires a
> review has been reviewed. I've done that now for 8247615 and I can
> also sponsor (i.e. push) the downport once it has been approved.
> 
> The general rules for 11u downports are described here:
> http://openjdk.java.net/projects/jdk-updates/approval.html
> http://openjdk.java.net/projects/jdk-updates/approval.html
> 
> Notice that the 8u updates project has recently introduced some new
> rules which I expect to get adopted for 11u as well:
> https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2020-
> August/003640.html
> 
> Best regards,
> Volker
> 
> On Tue, Aug 25, 2020 at 9:58 PM Jean Christophe Beyler
> <jcbeyler at google.com> wrote:
> >
> > Hi all,
> >
> > Does anybody have time to review this and help push it in per chance? Or
> am I missing something that I need to do :-) ?
> >
> > That would be much appreciated!
> >
> > Thanks again!
> > Jc
> >
> > On Fri, Aug 7, 2020 at 2:24 PM Jean Christophe Beyler
> <jcbeyler at google.com> wrote:
> >>
> >> Hi Volker,
> >>
> >> Sorry for the delay in my answer!
> >>
> >> First off, thanks for the review! I think I need someone to help push this
> webrev though and I am not sure if I need a second reviewer. That being
> said, here is the request with the empty line removed:
> >>
> >> - The original bug is here: https://bugs.openjdk.java.net/browse/JDK-
> 8247615
> >> - The original change is here:
> https://hg.openjdk.java.net/jdk/jdk/rev/13ec613f9373
> >> - The webrev with the new change:
> http://cr.openjdk.java.net/~jcbeyler/8247615/webrev.03_11/
> >> - The changes are not significant, I had just a few issues due to changes
> that came in between to the ThreadHeapSampler class; the test being new is
> unchanged
> >>
> >> Thanks for all your help, let me know if I need another reviewer or if
> someone can help push it for me!
> >>
> >> Thanks again,
> >> Jc
> >>
> >>
> >> On Wed, Jul 29, 2020 at 2:37 PM Volker Simonis
> <volker.simonis at gmail.com> wrote:
> >>>
> >>> Hi Jc,
> >>>
> >>> the downport looks fine to me except the extra empty line you've
> >>> inserted in threadHeapSampler.hpp:
> >>>
> >>>      _collectors_present = 0;
> >>> +
> >>> +
> >>> +    // Call this after _rnd is initialized to initialize _bytes_until_sample.
> >>>
> >>> Thank you and best regards,
> >>> Volker
> >>>
> >>> PS: as a general rule of thumb, a RFR for a downport should include
> >>> the following:
> >>> - a link to the original bug (JBS link)
> >>> - a link to the original change (i.e. a link to the Mercurial change
> >>> set, not to the original webrev)
> >>> - a link to the webrev with the new change
> >>> - if your changes to "original change" are significant an explanation
> >>> why they are required. A diff in webrev format of the "new change"
> >>> against "original change" might be helpful in such a case.
> >>>
> >>> On Wed, Jul 29, 2020 at 8:09 PM Jean Christophe Beyler
> >>> <jcbeyler at google.com> wrote:
> >>> >
> >>> > Hi all,
> >>> >
> >>> > Any chance to get a review on this? Did I perhaps missed something?
> >>> >
> >>> > Thanks again for your help!
> >>> > Jc
> >>> >
> >>> > On Thu, Jul 23, 2020 at 10:43 AM Jean Christophe Beyler
> <jcbeyler at google.com>
> >>> > wrote:
> >>> >
> >>> > > Hi all,
> >>> > >
> >>> > > First time I request a backport so if I am missing anything, please let
> me
> >>> > > know :)
> >>> > >
> >>> > > I am trying to port this webrev into 11u:
> >>> > >
> >>> > > Summary:
> >>> > > This fixes a sampling bias for the first allocations and ensures that the
> >>> > > sampling rate is correct from the start.
> >>> > > It is safe to apply as the whole code is protected behind flags and
> only
> >>> > > is used when JVMTI enables the heap monitoring.
> >>> > > We added a test that tickles this code on purpose.
> >>> > >
> >>> > > Porting issues:
> >>> > > Finally, the patch does not apply cleanly due to slight differences
> with
> >>> > > the files. This webrev provides the clean to JDK11u:
> >>> > > http://cr.openjdk.java.net/~jcbeyler/8247615/webrev.02_11/
> >>> > >
> >>> > > - That webrev compiles and tests for HeapMonitor (with the new
> test) pass:
> >>> > >     make run-test
> >>> > > TEST="test/hotspot/jtreg/serviceability/jvmti/HeapMonitor"
> >>> > >
> >>> > > compared to what I submitted to head:
> >>> > > http://cr.openjdk.java.net/~jcbeyler/8247615/webrev.02/
> >>> > >
> >>> > > Let me know what else I can do or provide!
> >>> > >
> >>> > > Thanks for all your help,
> >>> > > Jc
> >>> > >
> >>> > > Ps: I sent this email before I subscribed so one is waiting in
> moderation;
> >>> > > if this shows up in double, I apologize :)
> >>> > >
> >>> >
> >>> >
> >>> > --
> >>> >
> >>> > Thanks,
> >>> > Jc
> >>
> >>
> >>
> >> --
> >>
> >> Thanks,
> >> Jc
> >
> >
> >
> > --
> >
> > Thanks,
> > Jc