RFR(S) 8210388 Use hash table to store archived subgraph_info records

[Jiangli Zhou]

Hi Ioi,

On 9/20/18 4:41 PM, Ioi Lam wrote:

>
>
> On 09/20/2018 03:07 PM, Jiangli Zhou wrote:
>> Hi Ioi,
>>
>>
>> On 9/20/18 11:28 AM, Ioi Lam wrote:
>>> http://cr.openjdk.java.net/~iklam/jdk12/8210388-hashtable-subgraph-info.v01/ 
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8210388
>>>
>>> Here's another small step towards CDS support for Lambda (JDK-8198698).
>>>
>>> Replace the linear search of ArchivedKlassSubGraphInfoRecord with
>>> a hashtable. Currently we have just 6 such records, but with Lambda 
>>> support
>>> that number is going to increase substiantially.
>>>
>>>      Shared subgraphs table stats
>>>      Number of entries       :         6
>>>      Total bytes used        :        72
>>>      Average bytes per entry :    12.000
>>>      Average bucket size     :     6.000
>>>      Variance of bucket size :     0.000
>>>      Std. dev. of bucket size:     0.000
>>>      Empty buckets           :         0
>>>      Value_Only buckets      :         0
>>>      Other buckets           :         1
>>>
>>> I also took the chance to clean up the use of CompactHashtableWriter in
>>> the symbol/string tables. In doing so, I fixed a bug where
>>> SymbolTable::copy_shared_symbol_table was skipping symbols with zero 
>>> hashcodes.
>>> This should be done for the string table only.
>>>
>>> Jiangli, could you help me with this comments?
>>>
>>>     unsigned int hash = java_lang_String::hash_code(s);
>>>     if (hash == 0) {
>>>       // We do not archive Strings with a 0 hashcode because ......
>>>       return true;
>>>     }
>> The intent was to prevent possible write into the closed archive 
>> region when identity_hash was called on an empty String. The empty 
>> String is archived in the open archive heap region. This may not be 
>> necessary as we always pre-compute identity hash for archived object. 
>> If you want to remove this restriction, please be careful to verify 
>> all possible cases.
>>
>
> Hi Jiangli,
>
> Thanks for the explanation. I am not going to change the logic in the 
> code quoted above. I just wanted to add a comment there to explain why 
> we are doing that. Do we have an existing comment for this else where?
Please do. Thanks. There is no existing comment, I think.
>
> I removed the (hash == 0) check in the symbol table dumping code, 
> since that's a bug.
I'll take a look of your change.

Thanks,
Jiangli
>
> Thanks
> - Ioi
>
>
>> Thanks,
>> Jiangli
>>>
>>>
>>> Thanks
>>> - Ioi
>>
>