RFR(S): 8087143: Reduce Symbol::_identity_hash to 2 bytes

[Yumin Qi]

Coleen,

   Thanks.

On 6/19/2015 5:14 AM, Coleen Phillimore wrote:
>
> Yumin,
>
> If the identity hash isn't unique enough you get a system dictionary 
> warning like this, which doesn't stop the JVM.   Could you check your 
> test logs to see if this warning is output in any of them (jtreg keeps 
> logs around, just checking the jtreg test results should be 
> sufficient, I think).
>
> Log : HotSpot(TM) Server VM warning: Performance bug: SystemDictionary 
> lookup_count=6918 lookup_length=3907 average=0.564759 load=0.266601
>
> I think with including _body[0] in the function you may make the hash 
> random enough.   So this was one of my concerns with this change.
>
I will check that.
> In 
> http://cr.openjdk.java.net/~minqi/8087143/webrev01/src/share/vm/runtime/java.cpp.udiff.html
>
> Are you sure you want all of the SymbolTable dump?  Maybe it should be 
> under PrintSymbolTableSizeStatistics instead?
>
I will check and make change based on the output of statistics, maybe it 
is too much to print out all SymbolTable.
> In 
> http://cr.openjdk.java.net/~minqi/8087143/webrev01/src/share/vm/oops/symbol.hpp.udiff.html
>
> -// We separate the fields in SymbolBase from Symbol::_body so that
> -// Symbol::size(int) can correctly calculate the space needed.
>
> I'm not sure what that comment meant.  With SymbolBase gone (which I 
> think looks nicer), can you still correctly calculate the space needed?
>
Yes, now the _body changed from 1 byte array to 2 bytes array.
The original calculation is not correct I think:  if symbol length > 0, 
we allocate *_ONE_* extra byte for it:

-    size_t sz = heap_word_size(sizeof(SymbolBase) + (length > 0 ? length : 0));
+    size_t sz = heap_word_size(sizeof(Symbol) + (length > 2 ? length - 2 : 0));


> Other than these things, this change looks good.  Nice to find some 
> space savings with something so frequent as symbols.
>
> Thanks,
> Coleen
>
> On 6/18/15 8:40 PM, Yumin Qi wrote:
>> Ioi,
>>
>>   Thanks for review, I will add that (for print out purpose, not in 
>> the final change?)
>>
>> Yumin
>>
>> On 6/18/2015 3:34 PM, ioi.lam at oracle.com wrote:
>>> Hi Yumin,
>>>
>>> I wonder why the average symbol size didn't change. They are both 
>>> about 34.25 bytes long.
>>>
>>> When I tested the change, -Xshare:dump -XX:+PrintSharedSpaces show 
>>> about 1.3 bytes reduction per symbol.
>>>
>>> Also, could you add some code to print out the statistics of the 
>>> system dictionary to show if there's any change in the hashtable 
>>> balance?
>>>
>>> Thanks
>>>
>>>> On Jun 18, 2015, at 1:46 PM, Yumin Qi <yumin.qi at oracle.com> wrote:
>>>>
>>>> Please review the small change for
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8087143
>>>> webrev: http://cr.openjdk.java.net/~minqi/8087143/webrev01/
>>>>
>>>> Summary: _identity_hash is an integer in Symbol (SymbolBase), it is 
>>>> used to compute hash bucket index by modulus division of table 
>>>> size. Currently in hotspot, no table size is more than 65535 so we 
>>>> can use short instead. For case with table size over 65535 we can 
>>>> use the first two bytes of symbol data to be as the upper 16 bits 
>>>> for the calculation but rare cases.
>>>>
>>>> This is a minor change, also eliminates SymbolBase which seems 
>>>> redundant.  Also add print out SymbolTable statistics under debug 
>>>> flag PrintSystemDictionaryAtExit.
>>>>
>>>> Tests: JPRT, Specjbb2005.
>>>>
>>>> The SymbolTable statistics for specjbb2005:
>>>>
>>>> Before Change:
>>>> SymbolTable statistics:
>>>> Number of buckets       :     20011 =    320176 bytes, avg 16.000
>>>> Number of entries       :     49914 =   1597248 bytes, avg 32.000
>>>> Number of literals      :     49914 =   1709688 bytes, avg 34.253
>>>> Total footprint         :           =   3627112 bytes
>>>> Average bucket size     :     2.494
>>>> Variance of bucket size :     2.483
>>>> Std. dev. of bucket size:     1.576
>>>> Maximum bucket size     :        10
>>>>
>>>> After Change:
>>>>
>>>> SymbolTable statistics:
>>>> Number of buckets       :     20011 =    320176 bytes, avg 16.000
>>>> Number of entries       :     49911 =   1597152 bytes, avg 32.000
>>>> Number of literals      :     49911 =   1709544 bytes, avg 34.252
>>>> Total footprint         :           =   3626872 bytes
>>>> Average bucket size     :     2.494
>>>> Variance of bucket size :     2.483
>>>> Std. dev. of bucket size:     1.576
>>>> Maximum bucket size     :        10
>>>>
>>>> There is no big change for the hashtable balance.
>>>>
>>>>
>>>> Thanks
>>>> Yumin
>>
>