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

[Volker Simonis]

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