RFR (S): 8073204: Determining the desired PLAB size adjusts to the the number of threads at the wrong place

[sangheon.kim]

Hi Ramki and Jesper,

Here's updated webrev which contains your comments.
- Renamed to '_desired_net_plab_sz'.
- Changed comments to 'Assumed to have'.

And this webrev is created after a rename of G1ParGCAllocBuffer to G1PLAB.

webrev:
http://cr.openjdk.java.net/~jwilhelm/8073204/webrev.02/

Thanks,
Sangheon


On 04/08/2015 01:28 PM, Sangheon Kim wrote:
> Hi Jesper,
>
> Thank you for the review.
>
> On 04/08/2015 12:57 PM, Jesper Wilhelmsson wrote:
>> Hi Sangheon,
>>
>> Looks good in general. Just minor comments:
>>
>> In parGCAllocBuffer.cpp
>>
>> +  // Assume to have 1 gc worker thread
>> +  size_t recent_plab_sz = used / (target_refills * 1);
>>
>> I don't see the point of keeping the "* 1" here, or the parenthesis. 
>> The comment already says that we assume one thread. I think the 
>> comment should say "Assumed to have" or "We assume that we have".
> Okay, I will fix this.
>
>>
>> Since you were discussing naming with Ramki I'll add that I don't see 
>> the point in saving two characters by shortening size to sz. I'd 
>> prefer if size was spelled out throughout the code. That may be too 
>> much to change in this patch, but if you decide to change the name 
>> anyway please consider this as well.
> Yes. Thomas and I discussed from other email about changing name. But 
> as you said, it's too much to cover within this CR so I would prefer 
> to separate to another CR.
>
> Thanks,
> Sangheon
>
>
>>
>> Thanks,
>> /Jesper
>>
>>
>> Sangheon Kim skrev den 6/4/15 23:40:
>>> Hi all,
>>>
>>> Please review this change to determine the desired PLAB size for 
>>> current gc
>>> worker threads.
>>>
>>> Currently we calculate an optimal PLAB size with current number of 
>>> gc workers.
>>> When the number of workers changes dynamically
>>> (-XX:+UseDynamicNumberOfGCThreads), the desired PLAB size returned(by
>>> desired_plab_sz()) is still tuned to the number of gc workers that 
>>> has been used
>>> previously.
>>>
>>> This change is first calculate the desired PLAB value for a single 
>>> gc worker and
>>> then return desired PLAB size according to the current number of 
>>> threads.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8073204
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sangheki/8073204/webrev.01
>>>
>>> Test:
>>> JPRT
>>>
>>> Thanks,
>>> Sangheon
>