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