RFR (M): JDK-8030177: G1: Enable TLAB resizing

[Jesper Wilhelmsson]

Hi,

Bengt Rutisson skrev 24/1/14 8:36 AM:
>
> Hi Jon,
>
> Thanks for looking at this!
>
> On 1/24/14 12:36 AM, Jon Masamitsu wrote:
>> Bengt,
>>
>> http://cr.openjdk.java.net/~brutisso/8030177/webrev.01/src/share/vm/memory/genCollectedHeap.cpp.frames.html
>>
>>
>> An opportunity to remove _gens[] in another place?  Since we
>> really only have two generations in GenCollectedHeap.
>>
>>  936 size_t GenCollectedHeap::tlab_used(Thread* thr) const {
>>  937   size_t result = 0;
>>  938   for (int i = 0; i < _n_gens; i += 1) {
>>  939     if (_gens[i]->supports_tlab_allocation()) {
>>  940       result += _gens[i]->tlab_used();
>>  941     }
>>  942   }
>>  943   return result;
>>  944 }
>
> I like this idea, but I would prefer to make this as a general change where we
> remove the generations array. For now I would prefer that
> GenCollectedHeap::tlab_used() looks as similiar at possible to the other methods
> in GenCollectedHeap, such as GenCollectedHeap::supports_tlab_allocation(),
> GenCollectedHeap::tlab_capacity() and GenCollectedHeap::unsafe_max_tlab_alloc().

I have a change where I merge GenCollectorPolicy with 
TwoGenerationalCollectorPolicy. In that work I also removed a lot of the 
imaginary support for more than two generations. It all ties together...

It's not done yet and currently it's not at the top of my priority list, but let 
me know if you think I should give this a few more hours and get it in sooner 
rather than later.
/Jesper

>
>>
>> Can tlab_used() get Thread *thr from myThread() instead
>> of passing it in?   It's somewhat confusing when a parameter
>> is passed in and then ignored a lot of the time.
>
> I agreee. Currently it is only MutableNUMASpace that actually use the thread
> parameter. Again, my personal view is that we should treat tlab_used() similarly
> to the other tlab_* methods and I would prefer to do changes like this as
> separate fixes.
>
> Thanks,
> Bengt
>
>>
>> Rest looks good.
>>
>> Jon
>>
>> On 01/23/2014 01:27 AM, Bengt Rutisson wrote:
>>>
>>> Hi all,
>>>
>>> I went through this change with Stefan Karlsson. Here is an updated webrev
>>> based on his suggestions:
>>>
>>> http://cr.openjdk.java.net/~brutisso/8030177/webrev.01/
>>>
>>> Here is a webrev with just the changes that Stefan suggested:
>>>
>>> http://cr.openjdk.java.net/~brutisso/8030177/webrev.01_delta/
>>>
>>> Thanks,
>>> Bengt
>>>
>>>
>>> On 2014-01-13 14:09, Bengt Rutisson wrote:
>>>>
>>>> Hi Thomas,
>>>>
>>>> On 1/13/14 1:14 PM, Thomas Schatzl wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, 2014-01-09 at 20:32 +0100, Bengt Rutisson wrote:
>>>>>> Hi everyone,
>>>>>>
>>>>>> Can I have a couple of reviews for this change?
>>>>>>
>>>>>> http://cr.openjdk.java.net/~brutisso/8030177/webrev.00/
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8030177
>>>>>>
>>>>>> Unlike the other collectors G1 did not use dynamic sizes for the TLABs
>>>>>> that the Java threads use for allocation. That means that the TLABs
>>>>>> when using G1 were all fixed size and very small. This patch adds
>>>>>> dynamic sizing of TLABs to G1 too. In performance testing this has
>>>>>> shown a 2-3% improvement across a range of benchmarks.
>>>>>>
>>>>>> One difference compared to other collectors is that the maximum TLAB
>>>>>> size in G1 has to be limited by the region size and not by the eden
>>>>>> size. Also, for the other collectors the used space for TLAB
>>>>>> allocation was deducted from the maximum TLAB size because this was
>>>>>> essentially what was left in eden. That is an assumption that happened
>>>>>> to work out, but it kind of breaks the abstraction. So, the method
>>>>>> CollectedHeap:: tlab_used() has been added to be more consistent about
>>>>>> the TLAB abstraction and to make it more straight forward to implement
>>>>>> in G1.
>>>>> threadLocalAllocBuffer.cpp:
>>>>>
>>>>> line 83: typo:  allcoation -> allocation
>>>>>        // These allcoation should ideally not be counted but since it is
>>>>> not possible
>>>>>
>>>>> Looks good. It's okay for me to just fix this without a re-review ;-)
>>>>
>>>> Thanks! I fixed the typo. :)
>>>>
>>>> Bengt
>>>>
>>>>
>>>>>
>>>>> Thomas
>>>>>
>>>>>
>>>>
>>>
>>
>