Some benchmarking and improvements to schema creation times and object allocations#2453
Conversation
| this.defaultDataFetcherFactory = codeRegistry.defaultDataFetcherFactory; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
This allows us to mark a code registry and know if it changes since we marked it.
| public Builder clearTypeResolvers() { | ||
| typeResolverMap.clear(); | ||
| return this; | ||
| return markChanged(); |
There was a problem hiding this comment.
All the mutative methods mark the builder as changed
| // they could have changed the code registry so rebuild it | ||
| GraphQLCodeRegistry codeRegistry = this.codeRegistryBuilder.build(); | ||
| builder.codeRegistry(codeRegistry); | ||
| }); |
There was a problem hiding this comment.
We have two schema transformers here - one for a visitor that might not have done anything
And one because they MAY have changed the code registry.
Another way to fix this would be to have a way to change a schema's code registry safely (since its really a object swap in not a tree rebuild - but thats not this does
There was a problem hiding this comment.
This turned up in JProfiler as a heavily invoked bit of code in schema builds
| this.breadcrumbs = ImmutableList.<Breadcrumb<T>>builderWithExpectedSize(parent.getBreadcrumbs().size() + 1) | ||
| .add(new Breadcrumb<>(this.parent.thisNode(), this.location)) | ||
| .addAll(parent.getBreadcrumbs()).build(); | ||
|
|
There was a problem hiding this comment.
This is more efficient in memory allocation and faster to boot
| graphQLSchema = dummyRoot.rebuildSchema(codeRegistry); | ||
| if (postTransformation != null) { | ||
| graphQLSchema = graphQLSchema.transform(postTransformation); | ||
| } |
There was a problem hiding this comment.
This is a BIG change - if the visitor didnt change anything and the code registry didn't change - then nothing has changed - its the same schema - so we return it direct
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
We can merge the netflix benchmarks in in another PR
bbakerman
added
the
performance
# Conflicts: # src/test/java/benchmark/SchemaBenchMark.java
| when: | ||
| def newSchema = SchemaTransformer.transformSchema(schema, fieldChanger) | ||
| then: | ||
| newSchema == schema |
There was a problem hiding this comment.
I think you want is or ===.
I suspect we made this error quite a lot tbh.
See https://docs.groovy-lang.org/latest/html/documentation/core-operators.html#_relational_operators
# Conflicts: # src/test/groovy/graphql/schema/SchemaTransformerTest.groovy
2 participants
I started by running things under JProfiler
I noticed some hotspots and then decided to have a look at them
I added benchmark to ensure I got progress
I then applied the Immutable list breadcrumb fix
and then I applied the SchemaDirectiveWiringSchemaGeneratorPostProcessing change
This is not earth shattering and we still do to many schema transforms in practice, but it's an improvement