Repository navigation
Makes the performance of the data fetcher better by caching the method lookups - #1013
Conversation
…rties -> class+method | field
…erformance of PropertyDataFetcher
|
Tried graphql-java from this PR and my benchmark project at rrva/hello-dataloader@ed80001 Much better response time behavior. Did a quick profile to show what is left, but I am happy as is.
|
| } | ||
|
|
||
| private String mkKey(Class clazz, String propertyName) { | ||
| return clazz.getCanonicalName() + "__" + propertyName; |
There was a problem hiding this comment.
Maybe base the cache key on something else than clazz.getCanonicalName() which seems to be slow due to being a native method?
https://bugs.openjdk.java.net/browse/JDK-5058117
consider if getName() is sufficient, since it seems to be cached.
There was a problem hiding this comment.
also, getCanonicalName seems to return null for anonymous inner classes:
package foo;
public class Foo {
interface Bar {
void baz();
}
public static void main(String[] args) {
System.out.println("class name:"+Foo.class.getName());
Bar b = new Bar() {
@Override
public void baz() { }
};
System.out.println("anon inner class name: "+ b.getClass().getName());
System.out.println("anon inner class canonicalName: "+b.getClass().getCanonicalName());
}
}
There was a problem hiding this comment.
Why not use a SimpleImmutableEntry as a stdlib included Pair/Tuple implementation for the key?
There was a problem hiding this comment.
Thanks - will change it to Class.getName()
| private static final ConcurrentMap<String, Field> FIELD_CACHE = new ConcurrentHashMap<>(); | ||
|
|
||
| /** | ||
| * PropertyDataFetcher caches the methods and fields that map from a class -> a property for runtime performance reasons. |
There was a problem hiding this comment.
javadoc fails on using -> in a comment, change it to to
…fetching-performance
4 participants
see #1012 and a related PR #1011