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

[Jean Christophe Beyler]

Agreed; missed a hg qrefresh; link is now updated:
http://cr.openjdk.java.net/~jcbeyler/8247615/webrev.01/

:)
Jc

On Mon, Jun 29, 2020 at 2:28 PM Man Cao <manc at google.com> wrote:

> 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
>>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200629/5ac926f0/attachment-0001.htm>