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

[Jean Christophe Beyler]

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