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

[Jean Christophe Beyler]

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