Replies: 4 comments 12 replies
-
The challenge is that to be truly async - you need to embrace an async mechanism. In our case with Java 8 as a minimum when we embraced an async design, we had CompletableFutures to embrace. We do more than just fetch "field values" that are CFs - we compose functionality together. For example
We need to asynchronously compose together those fields. And CF.allOF() et al is how we do that. If we had a list of values, where some are sync objects and some are not then we would make the code a nightmare in terms of trying to compose together steps we make. Other code like In short CF is FUNDAMENTAL to how graphql-java works right now. We really cant avoid it now given the choices we have made. We would be breaking both API and semantics if we moved to a half sync values / half CFs say.
The fact is in a real world production system - resolving values in sync (I think you meant synchronously and in process) is not likely. Useful real world apps hit databases and other services and these are inherently async mechanisms. So yes in your test it feels pointless - but I posit it's not a real world application and hence exaggerates the drawbacks from the benefits.
This is doable. However you would be on your own for now. We have found that truly composeable ExecutionStratgeies are impossible - namely because the design choices are embedded into code. A coroutines ES would be designed differently and should be. This also has flow on effects to callbacks like In the medium term future, there is JVM virtual threads. Basically I think it will kill the async patterns INCLUDING (probably) coroutines as they exist today. We have done some POCs on this and it involves a new ES engine as well. So at some time graphql-java is likely to embrace virtual threads. But we wont until they become fully GA and widely used. And Java 19 is preview say. |
Beta Was this translation helpful? Give feedback.
-
hello @rrva I started to tackle this problem too! Please have a look here #3233, it is not much but maybe a good starting point. I see also other point that could be improved:
D. |
Beta Was this translation helpful? Give feedback.
-
Great stuff! I have not yet benchmarked these changes on my workload but this work is welcome, thanks @dfa1! I implemented a custom execution strategy in kotlin using coroutines and no CompletableFutures at all, but currently it performs far worse than stock AsyncExecutionStrategy in throughput benchmarks. It does not support instrumentations at all. Can share that code but this main approach looks much more promising and has a better upgrade path etc etc. |
Beta Was this translation helpful? Give feedback.
-
#3236 adding few more commits, please have a look when you have time |
Beta Was this translation helpful? Give feedback.
-
When profiling graphql-java and focusing on query-time allocations, CompletableFuture comes out at the very top. This is due to that each resolved field allocates at least one CompletableFuture, regardless of if the DataFetcher is async or not, this is just the very nature of how AsyncExecutionStrategy is implemented. Also there seems to be many intermediate CompletableFuture objects allocated, where there are several thenApply() etc steps.
During a 5 minute load test using queries which are completely resolved locally in-process, using java flight recording, and allocation profiling enabled I get the following allocation stats (18,9% of all allocations are CompletableFuture instances).
Some ideas:
Rewrite AsyncExecutionStrategy and base classes to produce less CompletableFuture objects per resolved field. The overall API I guess needs to be CF based even for fields that can be resolved synchronously but at least the amount of temporary intermediate CFs could be reduced.
Since we run Kotlin, and I guess several other graphql-java users do, one possible area of improvement is to implement an ExecutionStrategy based on kotlin coroutines, which would need less CF objects being allocated at least internally inside the execution strategy.
What do you think?
Beta Was this translation helpful? Give feedback.
All reactions