RFR: 8176593: Throwable::getStackTrace performance regression

[Jiangli Zhou]

Hi Claes,

Looks good! Thank you for making the changes.

I just noticed you used 4-space for indentation at line 212 and 231. Hotspot coding style is 2-space for indentation.

Thanks!
Jiangli

> On Mar 14, 2017, at 11:09 AM, Claes Redestad <claes.redestad at oracle.com> wrote:
> 
> Hi Jiangli,
> 
> On 2017-03-14 18:22, Jiangli Zhou wrote:
>> Hi Claes,
>> 
>> Thank you for looking into this! The changes look good to me. I have
>> some small suggestions below.
>> 
>> I’d suggest to add a comment explaining the
>> java_lang_String::hash_code() call before lookup_shared(). For example,
>> “shared table requires java_lang_String::hash_code()”.
> 
> Sure!
> 
>> 
>> How about changing the following hash_string() calls to
>> AltHashing::murmur3_32() directly? That would avoid the second
>> use_alternate_hashcode() check in hash_string().
>> 
>> 206 if (use_alternate_hashcode()) {
>> 207 hash = hash_string(name, len);
>> 208 }
>> 
>> 224 if (use_alternate_hashcode()) {
>> 225 hashValue = hash_string(name, len);
>> 226 }
> 
> I'm not so sure manually inlining matters here, but I haven't inspected
> the disassembly, but with a little refactoring and it arguably makes the
> intent clearer:
> 
> http://cr.openjdk.java.net/~redestad/8176593/hotspot.02/
> http://cr.openjdk.java.net/~redestad/8176593/hotspot.01.02.diff/
> 
> Thanks!
> 
> /Claes
> 
>> 
>> Thanks,
>> Jiangli
>> 
>>> On Mar 14, 2017, at 9:57 AM, Claes Redestad <claes.redestad at oracle.com
>>> <mailto:claes.redestad at oracle.com>> wrote:
>>> 
>>> Hi,
>>> 
>>> current implementation of StringTable::lookup_shared computes hash
>>> values twice even when the same hash algorithm is used, which stand out
>>> in profiles as the primary cause for a performance regression on things
>>> like Throwable::getStackTrace.
>>> 
>>> By refactoring the logic to not compute hashes using a specific
>>> algorithm more than once we recuperate the regression and actually beat
>>> JDK 8 on reported microbenchmarks.
>>> 
>>> Webrev: http://cr.openjdk.java.net/~redestad/8176593/hotspot.01/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8176593
>>> 
>>> Testing: RBT jdk-tier1,hs-runtime-pre-checkin,hs-tier2
>>> 
>>> Thanks!
>>> 
>>> /Claes
>>> 
>>