Replace Reflection usage with MethodHandle (3.0)

[re-thc]

MethodHandle is available since Java 7, so API wise shouldn't be a problem.

I've been reading the discussion (can't find the issue now) about json benchmarks and how jackson already does many things dsljson etc does in terms of optimizations and was just thinking what else could potentially be done to bridge the gap (out of curiosity). 1 thing I noticed is reflection is heavily used and MethodHandle could come in as a source of improvement.

Main motivation: Afterburner is currently crippled (if not broken) in Java 9 onwards, so would be good if there was some way to bridge this performance loss without major changes.

See https://stackoverflow.com/questions/22244402/how-can-i-improve-performance-of-field-set-perhap-using-methodhandles/22337726#22337726 for more information.

Thanks!

[cowtowncoder]

@hc-codersatlas Do you think MethodHandle is more efficient to use than Method? I think I had tested it briefly at some point, and not finding much difference, but would be happy to be proven wrong.

As to versioning... this is bit tricky, since although Jackson 2.7 and above do require Java 7 to compile, we do not currently require anything beyond Java 6 for running Jackson 2.x.
Although to be honest I am not sure how widely that is used, or whether it strictly works for all features (everything is loaded dynamically, but I do not have Java 6 JVM installed locally to verify).

So perhaps this could be done for 2.10, if there are indications there are benefits; either regarding performance or forwards-compatibility with Java 9.

[cowtowncoder]

One note on SO answer: I think the reason I saw no benefit was since Jackson actually forcibly disables access checks, so no dynamic checks are made. So the main benefit (if so) would be that no calls would be needed except to work around access limitations (which will also be problematic no matter what, I think, on Java 9).

[re-thc]

MethodHandle is more efficient. It's the engine behind lambda and other invokedynamic uses. Hotspot / Java optimized MethodHandle rigorously from Java 8 onwards 9as per SO post). Anything MethodHandle / Varhandle / AtomicFieldUpdater etc gets trusted by Hotspot to do constant folding (as of 8/9). Under the hood a lot of it just compiles down to Unsafe calls without using Unsafe (or intrinsics). Lambda came out in 8 so if you tested in using 7 that might be the difference.

It's the long term option in terms of moving forward too - reflection is a broken mess from 9 onwards.

[IrvingZhao]

@cowtowncoder Method.invoke 默认配置下，会在第16次调用时，生成GeneratedMethodAccessorXXX类，并构建对象。这样做的目的是为了加快Method.invoke的执行效率。
但是这会导致Java的PermGen持续增长并导致OOM。
在使用MethodHandle时，则不会创建这个类。
所以，个人觉得，可以考虑将Method.invoke切换成MethodHandle.invoke

My English is not good, so I just translate above with machine.

Under the default configuration of Method. invoke, the GeneratedMethod AccessorXXX class is generated and the object is constructed on the 16th call. The goal is to speed up the execution of Method. invoke.
But this will lead to a sustained growth of Java's PermGen and OOM.
When using MethodHandle, this class is not created.
So, I think I can consider switching Method. invoke to MethodHandle. invoke.

[IrvingZhao]

This is the test code.

public class Demo {
      private void method(Integer i) {
          System.out.println("method === " + i);
      }
  }

public void test(){
    Demo d = new Demo();
    MethodHandle handle = MethodHandles.lookup().findVirtual(Demo.class, "method",MethodType.methodType(void.class, Integer.class));
        for (int i = 0; i < 20; i++) {
            handle.invoke(d, i);
        }

        Method method = Demo.class.getDeclaredMethod("method", Integer.class);
        for (int i = 100; i < 120; i++) {
            method.invoke(d, i);
        }
}

output with jvm param -XX:+TraceClassLoading

method === 0
method === 1
method === 2
method === 3
method === 4
method === 5
method === 6
method === 7
method === 8
method === 9
method === 10
method === 11
method === 12
method === 13
method === 14
method === 15
method === 16
method === 17
method === 18
method === 19
method === 100
method === 101
method === 102
method === 103
method === 104
method === 105
method === 106
method === 107
method === 108
method === 109
method === 110
method === 111
method === 112
method === 113
method === 114
[0.393s][info   ][class,load] jdk.internal.reflect.ClassFileConstants source: jrt:/java.base
[0.393s][info   ][class,load] jdk.internal.reflect.AccessorGenerator source: jrt:/java.base
[0.393s][info   ][class,load] jdk.internal.reflect.MethodAccessorGenerator source: jrt:/java.base
[0.393s][info   ][class,load] jdk.internal.reflect.ByteVectorFactory source: jrt:/java.base
[0.393s][info   ][class,load] jdk.internal.reflect.ByteVector source: jrt:/java.base
[0.393s][info   ][class,load] jdk.internal.reflect.ByteVectorImpl source: jrt:/java.base
[0.394s][info   ][class,load] jdk.internal.reflect.ClassFileAssembler source: jrt:/java.base
[0.394s][info   ][class,load] jdk.internal.reflect.UTF8 source: jrt:/java.base
[0.394s][info   ][class,load] jdk.internal.reflect.Label source: jrt:/java.base
[0.394s][info   ][class,load] jdk.internal.reflect.Label$PatchInfo source: jrt:/java.base
[0.394s][info   ][class,load] jdk.internal.reflect.MethodAccessorGenerator$1 source: jrt:/java.base
[0.395s][info   ][class,load] jdk.internal.reflect.ClassDefiner source: jrt:/java.base
[0.395s][info   ][class,load] jdk.internal.reflect.ClassDefiner$1 source: jrt:/java.base
[0.395s][info   ][class,load] jdk.internal.reflect.GeneratedMethodAccessor1 source: __JVM_DefineClass__
method === 115
method === 116
method === 117
method === 118
method === 119

@Getter
@Setter
@AllArgsConstructor
@NoArgsConstructor
public static class DemoA {
    private String s1;
}

output of exec ObjectMapper.writeValueAsString(new DemoA("s1"));

{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
[1.463s][info   ][class,load] jdk.internal.reflect.GeneratedMethodAccessor2 source: __JVM_DefineClass__
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}
{"s1":"s1"}

Looking forward to your reply.

[cowtowncoder]

At this point anyone who wants to propose changes to use MethodHandle is free to do so: I do not have time to investigate that approach at this point, but could help with integration.
Timeline-wise I think it would make most sense initially for 3.0 (master), and if all went well, could be considered an option (at first) for next 2.x after 2.10, if one is to be released (so most likely 2.11).

Alternatively this could also work as part of Afterburner module, initially; I think there are plans by @stevenschlansker to prototype it.

[stevenschlansker]

Switching from classic Reflection to MethodHandle usage is a slightly different problem from either Afterburner or possible future code generators like my Blackbird.

The potential benefit, as @IrvingZhao highlights above, is avoiding costs of inflating GeneratedMethodAccessor handles, and instead uses the new JVM invokedynamic instruction that is built for this use case. In theory this will unlock better performance down the road, and it has a better access control model for moving forward into modulepath based deployments.

The drawback is that accessing private fields and methods in the same was as setAccessible is a little tricky: the necessary API wasn't available until Java 9, so you have to use some unfortunate hacks to run on Java 8 (which in practice work just fine, and aren't really any "worse" than the setAccessible call was in the first place, see https://github.com/FasterXML/jackson-modules-base/pull/85/files#diff-80165cd10442d698e37a11526970fd58 )

It would be very interesting to prototype this change and then compare performance.
I might find some time to do so. What's the approximate timeframe for initial 3.0 releases?

My personal prediction is that we find on modern JVMs it hardly matters at all: vanilla classic reflection vs vanilla MethodHandle vs Afterburner vs Blackbird, none of them delivers noticesable performance increases in the context of even a single database call or WAN HTTP request. But I still think the work is worth doing to use more modern approaches with possible future benefits to modulepath users or graal substratevm ahead-of-time fanatics.

[cowtowncoder]

Makes sense. So, on 3.0: I am focusing now on 2.10, to get it out the door. After that I could go back to 3.0 development -- so far most work has been with scaffolding, API changes (builder pattern), removing deprecated things, making things immutable.
But there are bigger things I want to rewrite, esp. wrt Creator/property introspection.
It is safe to say that no version of 3.0 will be released during 2019, and probably first release candidates could be mid-2020 (May?), so there is plenty of time to do bigger changes.

It may even be possible/desireable to move past Java 8 as baseline (if so, 11 being LTR), but that's different discussion to have.

[stevenschlansker]

As of Java 18, there no longer is a historical Reflection api implementation at all, it simply becomes calls into MethodHandles, but slightly less efficient. So we definitely should migrate:

https://openjdk.java.net/jeps/416

[cowtowncoder]

Oh fuck.

[GedMarc]

ASM is now bundled/released just before JRE and is bundled with the JDK now though - ? My understanding is that it is better to avoid reflection and proxy class into your encapsulated module-space so consuming libraries dont need to opens to jackson.databand
no more illegal reflection?

[cowtowncoder]

I suppose I am not quite clear on what Reflection api implementation here specifically means. It sounds like some aspects might be removed? I wouldn't think all basic Field, Method etc types and accessors could be removed either at all, or at very least not quickly.

Quick glance at linked-to article seems to suggest this is implementation change, not API.
Which would mean it'd make sense, over time, to move on to directly use MethodHandles etc. But not that change must be done soon or such.

[re-thc]

Nothing will be removed at the API level. The reflection API's backing implementation will be replaced. It might be slower but won't break.

[stevenschlansker]

I'm hacking around on this here: https://github.com/FasterXML/jackson-databind/compare/master...stevenschlansker:methodhandle-hacking?expand=1

39 failing tests to go...

[franz1981]

Adding a comment here, as noted in the mentioned JEP there won't be any performance advantage (apart from saving inflation, which is still a concern!) If the Field/Methods are saved (and called by) into non static final fields.
Only static final ref to such deliver the expected performance speedup (the same for MethodHandle afaik)

Quoting the relevant part of the JEP, FYI

For optimal performance, Method, Constructor, and Field instances should be held in static final fields so that they can be constant-folded by the JIT. When that is done, microbenchmarks show that the performance of the new implementation is significantly faster than the old implementation, by 43–57%.

When Method, Constructor, and Field instances are held in non-constant fields (e.g., in a non-final field or an array element), microbenchmarks show some performance degradation. The performance of field accesses is significantly slower than the old implementation, by 51–77%, when Field instances cannot be constant-folded.

This degradation may, however, not have much effect on the performance of real-world applications. We ran several serialization and deserialization benchmarks using real-world libraries and found no degradation in

• A custom JSON serialization and deserialization benchmark using Jackson,
• An XStream converter type benchmark, or
• A Kryo field serializer benchmark.

Currently the only unsafe but faster way to perform field's get without any static final references, is by using Unsafe to retrieve the Field's offset and using the appropriate Unsafe method to read the primitive field or an object one.
This latter as been deprecated for remove by java 18 but is still there :P

[pjfanning]

@franz1981 thanks for the detail. I suspect that there will no be no major driver for change until a JDK upgrade breaks the existing code. Then we will need to consider a change, probably involving a Multi-Release approach where for older JDKs things stay the same and for newer ones, we use MethodHandles or whatever works for that JDK.

[cowtowncoder]

Existing code does not use Unsafe in any way tho, so that wouldn't be a worry.

Use of static final field MethodHandle (etc) might be relevant for Afterburner, although maybe Blackbird already uses it.

[cowtowncoder]

Fixed via #5046