RFR #2 (S) CR 8015270: @Contended: fix multiple issues in the layout code

[David Holmes]

On 26/05/2013 3:08 AM, Aleksey Shipilev wrote:
> On 05/25/2013 06:13 PM, Coleen Phillimore wrote:
>>
>> This code still looks fine.   One comment, not a suggested change or
>> request to see this again.
>>
>> +  // TODO: We add +1 to always allocate non-zero resource arrays; we need
>> +  // to figure out if we still need to do this.
>>     int* nonstatic_oop_offsets;
>>     unsigned int* nonstatic_oop_counts;
>>     unsigned int nonstatic_oop_map_count = 0;
>> +  unsigned int max_nonstatic_oop_maps  = fac->count[NONSTATIC_OOP] + 1;
>>
>>     nonstatic_oop_offsets = NEW_RESOURCE_ARRAY_IN_THREAD(
>> -            THREAD, int, nonstatic_oop_count + 1);
>> +            THREAD, int, max_nonstatic_oop_maps);
>>     nonstatic_oop_counts  = NEW_RESOURCE_ARRAY_IN_THREAD(
>> -            THREAD, unsigned int, nonstatic_oop_count + 1);
>> +            THREAD, unsigned int, max_nonstatic_oop_maps);
>>
>>
>> I don't think there's anything to do here.  The code makes it clear
>> because we allocate these arrays unconditionally, even if the count of
>> nonstatic oops are zero.   NEW_RESOURCE_IN_ARRAY probably doesn't like that.
>
> Understood. However, Vladimir has a vague memory about that, and the
> off-the-wall tests are not failing without +1. Given we care about the
> footprint, it makes a perfect sense to squeeze a few bytes of the
> residual footprint.
>
> But glancing over the HS codebase, I see no usages for N_R_A_I_T with
> possible zero argument; it might as well mean we just need to document
> this in the macro comment, so others would have no questions about that.

Tracing through the code if you get to Amalloc and pass zero then I 
think things will break because it will return the address of the next 
empty slot. The next call to Amalloc with a non-zero value will have the 
exact same address returned!

David
-----

> -Aleksey.
>