RFR (S): 8067341: Modify PLAB sizing algorithm to waste less

[Eric Caspole]

Hi Thomas,
Maybe you already got reviews for v2 but I noticed some typos:

In this part it should be the G1LastPLABAverageOccupancy, right?

   57     // Calculate the new PLAB size as the amount of space that 
could be wasted to
   58     // keep TargetPLABWastePct given latest memory usage and that 
the last buffer
   59     // will be G1PLABPercent full.


Mangled sentence:

   68     // The amount of allocation performed is we can spend is 
independent of the

Otherwise looks good.
Thanks,
Eric



On 08/26/2015 08:29 AM, Thomas Schatzl wrote:
> Hi all,
>
>    I tried to get some more internal comments on the question of the
> final change to improve PLAB sizes.
>
> On Wed, 2015-08-26 at 11:54 +0200, Thomas Schatzl wrote:
>> Hi Jon (and Eric),
> [...]
>>> When _region_end_waste is high, waste_used_for_calculation under
>>> estimates the
>>> amount used so under estimates the amount of waste (since the waste
>>> is a percentage of waste_used_for_calculation ) and so lowers the size
>>> of the next PLAB.  I can see that it scales reasonably.
>>>
>>> Why G1ExpectedAveragePLABOccupancyPercent that can be different than 50?
>>>
>>> If it is so that the user can scale the PLAB's up or down, what about
>>
>> That is the intent.
>>
>>> scale factor called G1PLABPercent?
>>
>> That name seems to me a bit too unspecific, I chose
>> G1PLABOccupancyFactor for this revision. Feel free to comment on this.
>
> Others commented on the name and the value range; mainly that a
> percentage from 1-100 (or something similar) seems more natural or at
> least common, also the use as a divisor in the calculation.
>
> Using a double instead of an integer allows more flexibility in the
> choice of values.
>
> As for the name, the argument came up that the G1PLABPercent suggestion
> is somewhat too undescriptive, and it is better to use a longer, more
> appropriate name like G1LastPLABAverageOccupancy (note that added "Last"
> in the name). Since we do not expect it to be used a lot, except experts
> that measured that there is an issue there previously, a longer name
> does not seem to be a big issue.
>
> http://cr.openjdk.java.net/~tschatzl/8067341/webrev.2
> http://cr.openjdk.java.net/~tschatzl/8067341/webrev.0_to_2 (change from baseline!)
>
> Thanks,
>    Thomas
>