RFR: 8161379: Force inline methods calling Reflection.getCallerClass
Hi, most @CallerSensitive methodscall Reflection.getCallerClass(), which turn out to have problematic performance characteristics when it fails to inline. Making @CallerSensitive imply @ForceInline actually works rather well across benchmarks, but didn't meet with approval from hotspot-compiler because it's a hack (unlike @ForceInline /s). Instead, here is a patch to explicitly @ForceInline those methods where it can plausibly be helping with performance: Webrev: http://cr.openjdk.java.net/~redestad/8161379/jdk.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8161379 Thanks! /Claes
Anyone? On 07/19/2016 08:16 AM, Claes Redestad wrote:
Hi,
most @CallerSensitive methodscall Reflection.getCallerClass(), which turn out to have problematic performance characteristics when it fails to inline.
Making @CallerSensitive imply @ForceInline actually works rather well across benchmarks, but didn't meet with approval from hotspot-compiler because it's a hack (unlike @ForceInline /s).
Instead, here is a patch to explicitly @ForceInline those methods where it can plausibly be helping with performance:
Webrev: http://cr.openjdk.java.net/~redestad/8161379/jdk.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8161379
Thanks!
/Claes
This looks good to me, Claes. I wouldn't mind to have a comment at @ForceInline line mentioning this is for Reflection.getCallerClass() optimization. But, it might be recoverable from the source control anyway. Thanks, -Aleksey On 08/05/2016 10:37 PM, Claes Redestad wrote:
Anyone?
On 07/19/2016 08:16 AM, Claes Redestad wrote:
Hi,
most @CallerSensitive methodscall Reflection.getCallerClass(), which turn out to have problematic performance characteristics when it fails to inline.
Making @CallerSensitive imply @ForceInline actually works rather well across benchmarks, but didn't meet with approval from hotspot-compiler because it's a hack (unlike @ForceInline /s).
Instead, here is a patch to explicitly @ForceInline those methods where it can plausibly be helping with performance:
Webrev: http://cr.openjdk.java.net/~redestad/8161379/jdk.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8161379
Thanks!
/Claes
On 08/05/2016 12:56 PM, Aleksey Shipilev wrote:
This looks good to me, Claes.
Thanks!
I wouldn't mind to have a comment at @ForceInline line mentioning this is for Reflection.getCallerClass() optimization. But, it might be recoverable from the source control anyway.
Sure, how about: @ForceInline // to ensure Reflection.getCallerClass optimization http://cr.openjdk.java.net/~redestad/8161379/jdk.02/ While source control history helps, I think it's alright with some additional verbiage here in case someone figures out a way to implement any of these without Reflection.getCallerClass (since we ideally don't want to @ForceInline in too many places) /Claes
Thanks, -Aleksey
On 08/05/2016 10:37 PM, Claes Redestad wrote:
Anyone?
On 07/19/2016 08:16 AM, Claes Redestad wrote:
Hi,
most @CallerSensitive methodscall Reflection.getCallerClass(), which turn out to have problematic performance characteristics when it fails to inline.
Making @CallerSensitive imply @ForceInline actually works rather well across benchmarks, but didn't meet with approval from hotspot-compiler because it's a hack (unlike @ForceInline /s).
Instead, here is a patch to explicitly @ForceInline those methods where it can plausibly be helping with performance:
Webrev: http://cr.openjdk.java.net/~redestad/8161379/jdk.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8161379
Thanks!
/Claes
On 08/05/2016 11:22 PM, Claes Redestad wrote:
On 08/05/2016 12:56 PM, Aleksey Shipilev wrote:
I wouldn't mind to have a comment at @ForceInline line mentioning this is for Reflection.getCallerClass() optimization. But, it might be recoverable from the source control anyway.
Sure, how about:
@ForceInline // to ensure Reflection.getCallerClass optimization
+1 -Aleksey
On Aug 5, 2016, at 1:22 PM, Claes Redestad <claes.redestad@oracle.com> wrote:
Looks fine. How much improvement do you see from the benchmark run with this patch? Mandy
Thanks! Up to 12-15x on some reflection micros, but it varies wildly. This change does not improve the best case performance, but rather eliminates a cause for degradation and high run to run variance. /Claes Mandy Chung <mandy.chung@oracle.com> skrev: (5 augusti 2016 14:55:59 GMT-07:00)
On Aug 5, 2016, at 1:22 PM, Claes Redestad <claes.redestad@oracle.com> wrote:
Looks fine.
How much improvement do you see from the benchmark run with this patch?
Mandy
-- Sent from my Android device with K-9 Mail. Please excuse my brevity.
participants (3)
-
Aleksey Shipilev
-
Claes Redestad
-
Mandy Chung