graph-gophers / graphql-go

GraphQL server with a focus on ease of use
BSD 2-Clause "Simplified" License
4.64k stars 491 forks source link

Optimized batch loading without blocking #628

Open hypirion opened 8 months ago

hypirion commented 8 months ago

This is more or less the same issue as gqlgen's https://github.com/99designs/gqlgen/issues/518

Right now, there doesn't seem to be any way to defer evaluation and loading of elements -- you're forced to load elements inside the resolver instead of passing something that can be loaded in batch by the GraphQL resolver itself (e.g. a dataloader thunk). That means the dataloader has to wait a predefined amount of time (by default 16 milliseconds if you use https://github.com/graph-gophers/dataloader) and you need one goroutine per resolver that waits on the loader.

If the method resolvers were allowed to return a thunk as well (func () (T, error)) or something more exotic, the server implementation could walk the query and gather all thunks. Then, when there is nothing more to expand but thunks, the server could evaluate the highest ones up in the tree and repeat the evaluation step once more. In that way, you reduce the number of goroutines needed and can immediately trigger the resolvers instead of having to wait for a timeout. This is what graphql-java does, except it also tracks what kind of loaders are in use, so that it only spawns a single "goroutine" per batch loader.

Is this something that'd be interesting to implement for graphql-go? If yes, we can probably think out the specifics for how it's supposed to be implemented. I'm imagining something that could work like Jens Neuer's Dataloader v3.0, except being backwards compatible with the current evaluation order when thunks aren't used.

pavelnikolov commented 7 months ago

@hypirion This is an interesting approach and I haven't thought about it. It's similar to the way the dataloader package handles loading.

Do you think that this can be implemented in a backwards compatible way? I'm curious about the performance improvements of such an implementations too.

vikstrous2 commented 2 months ago

I ran into this article, which seems to talk about the same thing. I'm very interested in removing the need for loaders.