CRR (S/M): 6994322: Remove the is_tlab and is_noref / is_large_noref parameters from the CollectedHeap interface

Stefan Karlsson stefan.karlsson at oracle.com
Thu Jun 9 13:35:48 UTC 2011


Hi Tony,

Looks good. Thanks for cleaning this up.

One nit. The following comment shouldn't refer to the removed is_tlab 
parameter anymore:

g1CollectedHeap.hpp
  437   // * All non-TLAB allocation requests should go to mem_allocate()
  438   //   and mem_allocate() should never be called with is_tlab == true.

StefanK

On 06/09/2011 03:46 AM, Tony Printezis wrote:
> Hi all,
>
> This is the one you've all been waiting for. :-) I'd like a couple of 
> code reviews for this change:
>
> http://cr.openjdk.java.net/~tonyp/6994322/webrev.0/
>
> It removes two parameters from the mem_allocate() method and updates 
> its implementations and uses. The two parameters are the following.
>
> a) is_tlab : indicates whether the allocation is for a TLAB or not. 
> However, given that all TLAB allocations go through the 
> allocate_new_tlab() method, all calls to mem_allocate() have is_tlab 
> == false. (Note: in G1 we have had an assert that checks that !is_tlab 
> for a few months now without any failures.) So, having the is_tlab 
> parameter seems pointless.
>
> b) is_noref (also called: is_large_noref) : indicates that the 
> allocation is for a large scalar object (and in particular: an array). 
> The large_typearray_limit() method specifies how large such an object 
> should be and, based on this value, the allocation goes through either 
> array_allocate() or large_typearray_allocate(), the only difference 
> between the two is the value of the is_noref parameter that's passed 
> to the allocate_routines. However, all mem_allocate() implementations 
> just ignore that parameter (we think it's some baggage we're still 
> carrying from the Train GC!!!).
>
> The changes for a) were trivial for G1 and the framework GCs but a bit 
> more involved for PS given that the is_tlab parameter is propagated 
> through a few methods and a VM operation. But note that none of those 
> are actually called from allocate_new_tlab(), so it's safe to assume 
> that they are only called for non-TLAB allocations. I did go over that 
> code very carefully to convince myself that this is the case and I 
> don't see any flaws. I would appreciate if someone else also does that 
> just in case...
>
> The changes for b) were trivial overall, given that the is_noref 
> parameter was not actually used by any of the GCs. So I just had to 
> remove the large_typearray_limit() and large_typearray_allocate()methods.
>
> I tested this workspace with our GCs and I haven't seen any issues. 
> And please let me know if there are any concerns about this.
>
> Tony
>




More information about the hotspot-gc-dev mailing list