Skip to content

Conversation

@bbakerman
Copy link
Member

@bbakerman bbakerman commented Oct 12, 2022

This PR is was inspired from some work done on https://github.com/jord1e/graphql-java-asm-datafetcher

While it does not use any of the code, it uses the some of the idea.

That is could you dynamically generate a class that helps you fetch directly via method calls instead of reflection Method.invoke() calls

This uses JavaAssist to generate a peer class for each source class.

The generated class looks much like this

public class GeneratedUniqueName implements ByteCodeFetcher {

public Object fetch(Object sourceObject, String propertyName) {
   if (sourceObject == null) { return null; }
   graphql.schema.bytecode.SettersAndVoidPojo source = (graphql.schema.bytecode.SettersAndVoidPojo) sourceObject;
   if("name".equals(propertyName)) {
      return source.getName();
   } else {
      return null;
   }
}
}

Calling a method directly is 1.7 times after than use the same method.invoke. That sounds like a lot however its very quick so in the schema of all graphql processing its not a lot.

Benchmark                               Mode  Cnt         Score         Error  Units
ByteCodeGen.measureViaGeneratedClass   thrpt   15  67263591.084 ± 1577315.087  ops/s
ByteCodeGen.measureViaReflectedMethod  thrpt   15  39037523.623 ±  975625.383  ops/s

However we do get some improvements in object heavy benchmarks like the introspection one


Benchmark                                      Mode  Cnt  Score   Error  Units
IntrospectionBenchmark.benchMarkIntrospection  avgt   15  0.668 ± 0.069   s/op

vs master

Benchmark                                      Mode  Cnt  Score   Error  Units
IntrospectionBenchmark.benchMarkIntrospection  avgt   15  0.708 ± 0.078   s/op

This needs a few things to be ready.

We would need to shade Java assist - we dont want to expose this out to others.

We need a JVM wide switch to turn this on or off. Like the graphql.schema.PropertyDataFetcherHelper#setUseSetAccessible flag we have today.

I think we might turn if off in a release and let people opt in and then maybe invert that on another release.

@arlampin
Copy link
Contributor

I wonder if using LambdaMetafactory would provide similar speedup vs. Method reflection but with vastly simpler implementation. Some blogs & benchmarks indicate that it's almost as fast as direct access:

…y certain properties and can indicate which ones
@He-Pin
Copy link

He-Pin commented Oct 13, 2022

I am using reflectasm for our internal graphql implementation
https://github.com/EsotericSoftware/reflectasm

I think you can try this one too

@lqc
Copy link

lqc commented Oct 13, 2022

FWIW, In my expirience, both Javassist and Reflectasm have problems with JDK 17+ support. The only known to me code-generation library usable on newest JVMs is ByteBuddy (https://github.com/raphw/byte-buddy). Both Hibernate and Spring moved to using ByteBuddy, so it's a safe choice.

@bbakerman
Copy link
Member Author

@arlampin

See #2985

@andimarek
Copy link
Member

Can we close that @bbakerman ?

@bbakerman
Copy link
Member Author

We wont be using byte code generation - its price is too high for the pay off

@bbakerman bbakerman closed this Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants