Skip to content

Consider dropping reflectasm dependency #390

@lburja

Description

@lburja

As reported in #369, when using graphql-java-tools with Java 9+, a warning is printed on startup: An illegal reflective access operation has occurred...

This is due to the reflectasm library being not fully compatible with newer versions of Java.
The bug is already reported to reflectasm since 2018 (EsotericSoftware/reflectasm#64), but there seems to be little interest in having it fixed. The author of reflectasm says in one of the comments:

Sorry I'm too busy with other things ...

which may also indicate that reflectasm is not being actively maintained anymore.

The good news is that graphql-java-tools depends only lightly on reflectasm. In fact, the library is used in only one place, in MethodFieldResolver.kt to invoke resolver methods.

What is the impact of using standard Java reflection instead of reflectasm?

  1. Performance?
    Reflectasm claims to be 50% faster for method invocation than java reflection. However, the benchmark was run on JDK 7. Once we run it on newer versions of Java, we see a completely different picture:

JDK7

JDK8

JDK11

  • in Java 8, the difference is only around 25%
  • in Java 11 reflection is actually faster than reflectasm. Maybe the official reflectasm benchmark is flawed (I have run it nevertheless multiple times). In any case the trend is clear and in practice I believe that there won't be any performance impact ...
  1. Cleaner stacktraces
    Reflectasm generates a bit cleaner stacktraces. This can be mitigated by catching the InvocationTargetException and re-throwing the cause (if it's a RuntimeException or an Error), or wrapping it as a runtime exception (if it's checked).

Before:

java.lang.RuntimeException: Expected
	at graphql.kickstart.tools.Query.throwsRuntimeException(EndToEndSpecHelper.kt:304)
	at graphql.kickstart.tools.QueryMethodAccess.invoke(Unknown Source)
	at graphql.kickstart.tools.MethodFieldResolverDataFetcher.get(MethodFieldResolver.kt:199)
	at graphql.execution.ExecutionStrategy.fetchField(ExecutionStrategy.java:272)
	...

After:

java.lang.RuntimeException: Expected
	at graphql.kickstart.tools.Query.throwsRuntimeException(EndToEndSpecHelper.kt:304)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at graphql.kickstart.tools.MethodFieldResolverDataFetcher.get(MethodFieldResolver.kt:258)
	at graphql.execution.ExecutionStrategy.fetchField(ExecutionStrategy.java:272)
	...   

The Java reflection stacktrace contains a few more lines, but it's clean enough nevertheless.

I will propose a pull request to remove reflectasm.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions