Batch query performance and context options

[jmccaull]

@oliemansm, recently I've been in discussion with one of the guys maintaining Java GraphQL about problems we had with performance when using batched requests. I'm not going to rehash all of it, the issue is here: graphql-java/graphql-java#1537, but the underlying problem is that, due to the way the futures are created and the dispatching instrumentation works, we saw a huge hit when switching from aliasing multiple queries together to batching multiple requests.
From what I can tell Apollo offers per query context or per http request context when enabling their batch extension. I'm assuming the per http request option allows the data loaders to batching between requests in that batch. Currently in the servlet, the context is shared between each request in the batch, but because join is called on each future and the dispatching instrumentation has no concept of waiting for a certain time (as it does in Apollo) or a certain number of futures to be created, none of the queries in the http request will be batched together. So it seems to me like a hybrid of the two Apollo options that isn't optimal. Ultimately we would like to enable behavior like Apollo batch with a per request context, but having it be configurable would be great.
The "enabler" I got from the GraphQL guys was the addition of the ability to set the execution id in the execution input. Using this it is pretty straight forward to create a custom dispatching instrumentation with per request state that can dispatch at optimal times. It is pretty simple to move the join call up to the BatchExecutionHandler by having the GraphQLQueryInvoker call executeAsync instead, but I don't see a simple way to inject this instrumentation.
My initial thought is to create a QueryInvoker interface, abstract some of it out and create a per request invoker and a per query invoker. Thoughts, concerns and/or questions?
Thanks!

[jmccaull]

The more I get into this the more I dislike the current set up. I think I'm going to end up refactoring a bit more than just the query invoker. I want to have a good mechanism to actually create a context per query vs per http request. I also want to clean up the GraphQL context object a bit as it feels weird to create multiple context objects each with a ref to the http request/response objects.
A number of the fields in the context are marked as depreciated, how opposed to breaking changes are you? Any particular reason to exclude lombok?
Thanks

[oliemansm]

Not opposed to breaking changes at all, but we should probably document migration steps required if it touches public api. The project would definitely benefit from some major refactoring. Just didn’t have any time myself yet, so any help would be much appreciated. And not opposed to Lombok either.

[jmccaull]

Awesome, thanks. I've opened a decent PR that I'm going to iterate on a little. I'll beef up the documentation when it gets more complete #192

[oliemansm]

Fixed by PR #192