Hi,
On 2016-12-01 22:25, Claes Redestad wrote:
On 12/01/2016 10:21 AM, Mandy Chung wrote:
On Dec 1, 2016, at 9:52 AM, Claes Redestad
<[email protected]> wrote:
Hi,
good suggestion, this tidies up a bit while not affecting score:
http://cr.openjdk.java.net/~redestad/8170595/webrev.02
I like this better. It may be useful to add a private isTopLevel
Class() for getSimpleBinaryName to call that will benefit
getSimpleName and isMemberClass?
Good idea, but this seems like an independent optimization we should
do as a separate follow-up, no?
Found that both isMemberClass and isLocalClass can be optimized in a
vein very similar to isAnonymousClass. isTopLevelClass helps
getSimpleBinaryName a bit, and makes sense to implement for
completeness. After deeper testing it's clear that one of the
costlier operations in this code is getDeclaringClass0, which may spend
plenty of time (~30ns/op for String.class) in the VM walking the
list of inner classes to find the enclosing class - if any.
A very small improvement can also be attained by restructuring code so
that we don't instantiate the EnclosingMethodInfo
unless it's needed, but keep the checking to keep semantically identical
behavior:
http://cr.openjdk.java.net/~redestad/8170595/webrev.03/
This brings significant improvements to some variants:
Baseline:
Benchmark Mode Cnt Score Error Units
Clazz.isAnonymousClass_Anon avgt 25 211.010 ± 22.974 ns/op
Clazz.isAnonymousClass_Regular avgt 25 132.198 ± 23.817 ns/op
Clazz.isMemberClass_Anon avgt 25 336.149 ± 43.215 ns/op
Clazz.isMemberClass_Regular avgt 25 92.502 ± 9.804 ns/op
Clazz.isLocalClass_Anon avgt 25 326.158 ± 34.051 ns/op
Clazz.isLocalClass_Regular avgt 25 32.086 ± 2.750 ns/op
Patch:
Benchmark Mode Cnt Score Error Units
Clazz.isAnonymousClass_Anon avgt 25 174.526 ± 16.292 ns/op
Clazz.isAnonymousClass_Regular avgt 25 33.136 ± 2.636 ns/op
Clazz.isMemberClass_Anon avgt 25 115.029 ± 9.464 ns/op
Clazz.isMemberClass_Regular avgt 25 85.706 ± 3.984 ns/op
Clazz.isLocalClass_Anon avgt 25 177.411 ± 22.927 ns/op
Clazz.isLocalClass_Regular avgt 25 31.940 ± 2.346 ns/op
Since the changes are all similar but still not too expansive, I'm
leaning towards just keeping this
change in one bug.
Thanks!
