Request for review: 7168280: Eliminate the generic signature index slot from field array for field without generic signature

[Jiangli Zhou]

Hi Dean,

Thanks for the further comments.

On 05/11/2012 06:32 PM, Dean Long wrote:
> How about using a typeArrayOop as your temporary storage in 
> parse_fields() instead of a u2 array?  That way you can use the 
> existing from_field_array() that does a bounds check.

Several approaches were attempted for parse_fields(). Initially a 
typeArrayOop was used. However measurements indicated memory increase 
instead of reduction with the approach. The temporary allocated 
typeArrayOops were not being GC'ed and caused memory leaks. As a second 
approach, the number of fields with generic signature was counted first 
so a typeArrayOop was allocated with the exact size. That approached 
required an extra iteration of all fields. The temporary resource array 
approach avoids the pre-parsing of all fields and also does not have 
memory leak.

>
> It is unfortunate that FieldInfo can't get generic signature 
> information anymore, forcing you to move more logic into the callers.

Fortunately most of the logic are hidden in the fieldStreams.

>
> Some of the code that iterates through all the fields could be more 
> efficient if you still had all_fields_count() so you could easily find 
> the generic signature information at the end.  One way to do that is 
> to store the total fields count at the end of the array only if the 
> array contains generic signature information.  You can tell if the 
> array contains generic signature information by the length.  If the 
> length % FieldInfo::field_slots == 0, then there is no generic 
> signature information, and all_fields_count() is computed like 
> before.  If length % FieldInfo::field_slots != 0 then get the fields 
> count from the end of the array.  You would need to pad the array with 
> an extra slot if the number of generic signature slots happened to be 
> a multiple of FieldInfo::field_slots.

I considered of recording the field count in the array. However it still 
uses extra 2 bytes for each class. I also played with the idea of using 
length % FieldInfo::field_slots as indicator of the existence of generic 
signatures. But if the number of fields is the multiply of 
FieldInfo::field_slots, length % FieldInfo::field_slots can be 0 if all 
fields have generic signatures.

I did some performance measurements with the field array changes. There 
seems no performance degradation.

============================================================================== 

logs.field_baseline.1:
   Benchmark           Samples        Mean     Stdev             Geomean 
Weight
   specjbb2005               8    56057.48    454.04
   specjvm98                 8      533.28     17.25
============================================================================== 

logs.field.2:
   Benchmark           Samples        Mean     Stdev   %Diff     P  
Significant
   specjbb2005               8    56321.18    614.18    0.47 
0.347            *
   specjvm98                 8      537.81     11.07    0.85 
0.544            *
============================================================================== 


Thanks again for the review and comments. Can I count you as a reviewer?

Jiangli

>
> dl
>
> On 5/11/2012 2:43 PM, Jiangli Zhou wrote:
>> Hi Dean,
>>
>> Here is the updated webrev 
>> http://cr.openjdk.java.net/~jiangli/7168280/webrev.01/. In 
>> init_generic_signature_start_slot(), it first scans from f[0] to 
>> f[_index - 1] so the generic signature slots for those fields can be 
>> skipped while scanning from f[_index] to the end.
>>
>> Thanks again,
>>
>> Jiangli
>>
>> On 05/11/2012 01:53 PM, Jiangli Zhou wrote:
>>> Hi Dean,
>>>
>>> Good catch! Thanks for the comments.
>>>
>>> Jiangli
>>>
>>> On 05/11/2012 01:38 PM, Dean Long wrote:
>>>> I think this breaks InternalFieldStream. The loop in 
>>>> init_generic_signature_start_slot() will start at 
>>>> java_fields_count() instead of 0, so it won't decrement "length" 
>>>> for any Java fields with a generic signature, resulting in scanning 
>>>> too many elements.
>>>>
>>>> dl
>>>>
>>>> On 5/11/2012 12:02 PM, Jiangli Zhou wrote:
>>>>> Hi,
>>>>>
>>>>> The field array is an array of 7-shorts [access, name index, sig 
>>>>> index, initval index, low offset, high offset, generic signature 
>>>>> index]. Most fields don't have generic signature attribute, in 
>>>>> that case the generic signature index in the field array is 0 and 
>>>>> unused. For all classes in rt.jar, there are total about 28973 
>>>>> fields. From which 27466 fields do not have generic signature and 
>>>>> only 1507 fields have generic signature.
>>>>>
>>>>> Following is a webrev that eliminates the unused generic signature 
>>>>> index slot from field array. For field without generic signature, 
>>>>> the field data is 6-shorts. For field with generic signature, it 
>>>>> is still 7-shorts of data. A new flag is added in the field access 
>>>>> flag to indicate if the field has the extra short for generic 
>>>>> signature index.
>>>>>
>>>>>   http://cr.openjdk.java.net/~jiangli/7168280/webrev.00/
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Jiangli
>>>
>>