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

[Volker Simonis]

Thanks Christoph!

Pushed.


On Thu, Aug 27, 2020 at 6:21 PM Langer, Christoph <christoph.langer at sap.com>
wrote:

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