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

[Martin Buchholz]

LGTM

On Tue, Aug 25, 2020 at 12:59 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