RFR: 8176593: Throwable::getStackTrace performance regression

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR: 8176593: Throwable::getStackTrace performance regression

Claes Redestad
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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

Jiangli Zhou
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()”.

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   }
Thanks,
Jiangli

> On Mar 14, 2017, at 9:57 AM, Claes Redestad <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

Claes Redestad
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 <[hidden email]
>> <mailto:[hidden email]>> 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
>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: 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 <[hidden email]> 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 <[hidden email]
>>> <mailto:[hidden email]>> 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
>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

Claes Redestad
Hi,

On 2017-03-14 19:20, Jiangli Zhou wrote:
> Hi Claes,
>
> Looks good! Thank you for making the changes.

Thanks!

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

I guess I'll never get used to that. Fixed. :-)

/Claes

>
> Thanks!
> Jiangli
>
>> On Mar 14, 2017, at 11:09 AM, Claes Redestad <[hidden email]> 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 <[hidden email]
>>>> <mailto:[hidden email]>> 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
>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

coleen.phillimore
In reply to this post by Claes Redestad

Hi Claes,
This is a good find and very nice that it improves performance with just
refactoring.

I think you could remove these (2):

+ if (use_alternate_hashcode()) {
+ hash = alt_hash_string(name, len);
+ }

Because there isn't a safepoint in lookup_shared, is there?  So the
hashing won't change.  Please correct me if I'm wrong.

When we add the string in basic_add, after taking the StringTable_lock,
we recheck the hashcode there.

I like your alt_hash_string function though, but I think it and
"hash_string" should be internal to stringTable.cpp (and not in the .hpp
file) because they're not public and only called from stringTable.cpp.

Thank you for fixing this!
Coleen

On 3/14/17 2:09 PM, Claes Redestad 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 <[hidden email]
>>> <mailto:[hidden email]>> 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
>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

serguei.spitsyn@oracle.com
In reply to this post by Claes Redestad
Hi Claes,

The fix looks good to me.

Nit:

+ // shared table always use java_lang_String::hash_code A typo: use =>
uses


Thanks,
Serguei



On 3/14/17 11:09, Claes Redestad 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 <[hidden email]
>>> <mailto:[hidden email]>> 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
>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

Jiangli Zhou
In reply to this post by coleen.phillimore
Hi Coleen,

> On Mar 14, 2017, at 12:28 PM, [hidden email] wrote:
>
>
> Hi Claes,
> This is a good find and very nice that it improves performance with just refactoring.
>
> I think you could remove these (2):
>
> + if (use_alternate_hashcode()) {
> + hash = alt_hash_string(name, len);
> + }
>
> Because there isn't a safepoint in lookup_shared, is there?  So the hashing won't change.  Please correct me if I'm wrong.

Claes is probably off line now, so I’m answering this one for him. :-) java_lang_String::hash_code computes without using the alternate hash method. That works for the shared table. If use_alternate_hashcode is enabled and the string cannot be found within the shared table, it needs to lookup the string in the runtime string table using the alternate hashcode. Otherwise, it could use the wrong hash value and fail to find the string.

Thanks,
Jiangli

>
> When we add the string in basic_add, after taking the StringTable_lock, we recheck the hashcode there.
>
> I like your alt_hash_string function though, but I think it and "hash_string" should be internal to stringTable.cpp (and not in the .hpp file) because they're not public and only called from stringTable.cpp.
>
> Thank you for fixing this!
> Coleen
>
> On 3/14/17 2:09 PM, Claes Redestad 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 <[hidden email]
>>>> <mailto:[hidden email]>> 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
>>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

coleen.phillimore


On 3/14/17 3:58 PM, Jiangli Zhou wrote:

> Hi Coleen,
>
>> On Mar 14, 2017, at 12:28 PM, [hidden email] wrote:
>>
>>
>> Hi Claes,
>> This is a good find and very nice that it improves performance with just refactoring.
>>
>> I think you could remove these (2):
>>
>> + if (use_alternate_hashcode()) {
>> + hash = alt_hash_string(name, len);
>> + }
>>
>> Because there isn't a safepoint in lookup_shared, is there?  So the hashing won't change.  Please correct me if I'm wrong.
> Claes is probably off line now, so I’m answering this one for him. :-) java_lang_String::hash_code computes without using the alternate hash method. That works for the shared table. If use_alternate_hashcode is enabled and the string cannot be found within the shared table, it needs to lookup the string in the runtime string table using the alternate hashcode. Otherwise, it could use the wrong hash value and fail to find the string.

I see.  Claes is online and tells me that alt_hash_string() can't be an
internal function because it calls seed().

So I think the change is good as-is.

thanks!
Coleen

>
> Thanks,
> Jiangli
>
>> When we add the string in basic_add, after taking the StringTable_lock, we recheck the hashcode there.
>>
>> I like your alt_hash_string function though, but I think it and "hash_string" should be internal to stringTable.cpp (and not in the .hpp file) because they're not public and only called from stringTable.cpp.
>>
>> Thank you for fixing this!
>> Coleen
>>
>> On 3/14/17 2:09 PM, Claes Redestad 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 <[hidden email]
>>>>> <mailto:[hidden email]>> 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
>>>>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

Claes Redestad


On 2017-03-14 21:39, [hidden email] wrote:
>
> So I think the change is good as-is.

Thanks Coleen, Jiangli and Serguei!

As we discussed offline, however, the new and existing hash_string
methods should be private:

http://cr.openjdk.java.net/~redestad/8176593/hotspot.03/

Also added the typo fix suggested by Serguei and updated copyrights.

Thanks!

/Claes

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

Mandy Chung

> On Mar 14, 2017, at 3:28 PM, Claes Redestad <[hidden email]> wrote:
>
> http://cr.openjdk.java.net/~redestad/8176593/hotspot.03/

Looks good.

This reminds me JEP 230 that we need to build up/collect microbenchmarks that we can do the performance runs to catch this kind of performance regressions during the development of a release.

Mandy

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

Claes Redestad


On 2017-03-14 23:39, Mandy Chung wrote:
>
>> On Mar 14, 2017, at 3:28 PM, Claes Redestad <[hidden email]> wrote:
>>
>> http://cr.openjdk.java.net/~redestad/8176593/hotspot.03/
>
> Looks good.

Thanks Mandy!

>
> This reminds me JEP 230 that we need to build up/collect microbenchmarks that we can do the performance runs to catch this kind of performance regressions during the development of a release.

Yes, this would be a great project to re-visit for JDK 10, and if JEP
296 is delivered a lot of the somewhat controversial aspects of JEP 230
would magically disappear.

/Claes

>
> Mandy
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

serguei.spitsyn@oracle.com
In reply to this post by Claes Redestad
Claes,

New webrev looks good.
Thank you for the update!

Thanks,
Serguei


On 3/14/17 15:28, Claes Redestad wrote:

>
>
> On 2017-03-14 21:39, [hidden email] wrote:
>>
>> So I think the change is good as-is.
>
> Thanks Coleen, Jiangli and Serguei!
>
> As we discussed offline, however, the new and existing hash_string
> methods should be private:
>
> http://cr.openjdk.java.net/~redestad/8176593/hotspot.03/
>
> Also added the typo fix suggested by Serguei and updated copyrights.
>
> Thanks!
>
> /Claes
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

coleen.phillimore
Looks good!
Coleen

On 3/14/17 6:51 PM, [hidden email] wrote:

> Claes,
>
> New webrev looks good.
> Thank you for the update!
>
> Thanks,
> Serguei
>
>
> On 3/14/17 15:28, Claes Redestad wrote:
>>
>>
>> On 2017-03-14 21:39, [hidden email] wrote:
>>>
>>> So I think the change is good as-is.
>>
>> Thanks Coleen, Jiangli and Serguei!
>>
>> As we discussed offline, however, the new and existing hash_string
>> methods should be private:
>>
>> http://cr.openjdk.java.net/~redestad/8176593/hotspot.03/
>>
>> Also added the typo fix suggested by Serguei and updated copyrights.
>>
>> Thanks!
>>
>> /Claes
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR: 8176593: Throwable::getStackTrace performance regression

Jiangli Zhou
+1

Thanks,
Jiangli

> On Mar 14, 2017, at 4:11 PM, [hidden email] wrote:
>
> Looks good!
> Coleen
>
> On 3/14/17 6:51 PM, [hidden email] wrote:
>> Claes,
>>
>> New webrev looks good.
>> Thank you for the update!
>>
>> Thanks,
>> Serguei
>>
>>
>> On 3/14/17 15:28, Claes Redestad wrote:
>>>
>>>
>>> On 2017-03-14 21:39, [hidden email] wrote:
>>>>
>>>> So I think the change is good as-is.
>>>
>>> Thanks Coleen, Jiangli and Serguei!
>>>
>>> As we discussed offline, however, the new and existing hash_string
>>> methods should be private:
>>>
>>> http://cr.openjdk.java.net/~redestad/8176593/hotspot.03/
>>>
>>> Also added the typo fix suggested by Serguei and updated copyrights.
>>>
>>> Thanks!
>>>
>>> /Claes
>>>
>>
>

Loading...