CRR (S/M): 6994322: Remove the is_tlab and is_noref / is_large_noref parameters from the CollectedHeap interface
Tony Printezis
tony.printezis at oracle.com
Thu Jun 9 18:10:05 UTC 2011
Thanks Stefan for the prompt code review! And thanks for catching the
issue below.
Here's the updated webrev:
http://cr.openjdk.java.net/~tonyp/6994322/webrev.1/
Tony
On 06/09/2011 09:35 AM, Stefan Karlsson wrote:
> 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