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

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Fri Jan 24 10:31:29 UTC 2014

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 sizet GenCollectedHeap::tlabused(Thread* thr) const { 937 sizet result = 0; 938 for (int i = 0; i < ngens; i += 1) {_ _939 if (gens[i]->supportstlaballocation()) { 940 result += gens[i]->tlabused(); 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::tlabused() looks as similiar at possible to the other methods in GenCollectedHeap, such as GenCollectedHeap::supportstlaballocation(), GenCollectedHeap::tlabcapacity() and GenCollectedHeap::unsafemaxtlaballoc().

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 tlabused() 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 tlabused() 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.01delta/ 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:: tlabused() 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

More information about the hotspot-gc-dev mailing list