RFR (S) 8247615: Initialize the bytes left for the heap sampler

[Man Cao]

Looks good.

> though adding the change that Man wants might make it more flaky so I
added your numThreads / 2 in case
I don't see the "numThreads / 2" in webrev.01 though. No need for a webrev
for this fix.

-Man


On Mon, Jun 29, 2020 at 1:10 PM Jean Christophe Beyler <jcbeyler at google.com>
wrote:

> Hi all,
>
> Sorry it took time to get back to this; could I get a new review from:
> http://cr.openjdk.java.net/~jcbeyler/8247615/webrev.01/
>
> The bug is here:
> https://bugs.openjdk.java.net/browse/JDK-8247615
>
> Note, this passed the submit repo testing.
>
> Thanks and have a great day!
> Jc
>
> Ps: explicit inlined Acks/Done are below:
>
> Sorry it took time to get back to this:
> @Martin:
>    - done the typo
>    - about the sampling test: No you won't get samples due to how the
> system is done, since we know we only will be allocating one object for the
> thread, it dies out before a sample is required... though adding the change
> that Man wants might make it more flaky so I added your numThreads / 2 in
> case
>   - done for the always in the description
>
>
> On Thu, Jun 25, 2020 at 6:54 PM Derek Thomson <dthomson at google.com> wrote:
>
>> > It could also avoid the problem where every thread deterministically
>> allocates the same object at 512K, although this is unlikely.
>>
>> I've recently discovered that with certain server frameworks that this
>> actually becomes quite likely! So I'd strongly recommend using
>> pick_next_sample.
>>
>
> Ack, done :)
>
>
>>
>> On Thu, Jun 25, 2020 at 4:56 PM Man Cao <manc at google.com> wrote:
>>
>>> Thanks for fixing this!
>>>
>>> > 53 ThreadHeapSampler() :
>>> _bytes_until_sample(get_sampling_interval())  {
>>>
>>> Does this work better? (It has to be done after the initialization of
>>> _rnd.)
>>> _bytes_until_sample = pick_next_sample();
>>>
>>> It could avoid completely missing to sample the first 512K allocation.
>>> It could also avoid the problem where every thread
>>>
>>
> Done.
>
>
>
>> deterministically allocates the same object at 512K, although this is
>>> unlikely.
>>>
>>> -Man
>>>
>>
>
> --
>
> Thanks,
> Jc
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200629/fbfa87cb/attachment-0001.htm>