Hi Christoph,

nothing wrong with tiny code changes when they actually matter (and
Class.isAnonymousClass is rather widely used internally and in reflection
libraries, so I'd say any significant improvement is worth considering).

I'm a bit surprised by the large improvement from your suggested
change though, since it seems the largest cost by far here seems to be
the call to getSimpleName(). Care to share your micro?

Looking at Class.isAnonymousClass it seems like we could do even better
by digging a bit deeper and breaking some rules of abstraction:

Current:

    public boolean isAnonymousClass() {
        return "".equals(getSimpleName());
    }

Clazz.isAnonymousClass  avgt    5  124.861 ± 37.689  ns/op

    public boolean isAnonymousClass() {
        return !isArray() && getEnclosingMethod0() != null &&
                getSimpleBinaryName0() == null;
    }

Clazz.isAnonymousClass  avgt    5  45.079 ± 12.238  ns/op

Thanks!

/Claes

Clazz.java:

package org.openjdk;

import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;

@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class Clazz {

    public static Class<?> c = String.class;

    @Benchmark
    public boolean isAnonymousClass() {
        return c.isAnonymousClass();
    }
}

On 2016-12-01 08:41, Christoph Dreis wrote:
Hey,

I'm currently getting familiar with the source code to eventually contribute
something more in the future. While doing so I noticed some smaller
enhancements where I don't know if they even justify a mail. Please let me
know how you handle such tiny improvements based on the following:

One of the arguably small improvements was Class.isAnonymousClass() which
checks for emptiness with "".equals(getSimpleName()) instead of
getSimpleName().isEmpty() and I see no way of getSimpleName() returning null
(I might miss something though). Anyhow, the latter is slightly faster and a
bit more verbose:

MyBenchmark.testEmpty   thrpt   20  364479649,385 ± 5805392,007  ops/s
MyBenchmark.testEquals   thrpt   20  287935443,484 ± 2895104,850  ops/s

Again - if this is too small please let me know and excuse the disturbance.

Cheers,
Christoph

=========== PATCH ============
# User Christoph Dreis <christoph.dr...@freenet.de>
Small enhancement for Class.isAnonymousClass()

diff --git a/src/java.base/share/classes/java/lang/Class.java
b/src/java.base/share/classes/java/lang/Class.java
--- a/src/java.base/share/classes/java/lang/Class.java
+++ b/src/java.base/share/classes/java/lang/Class.java
@@ -1596,7 +1596,7 @@
       * @since 1.5
       */
      public boolean isAnonymousClass() {
-        return "".equals(getSimpleName());
+        return getSimpleName().isEmpty();
      }


Reply via email to