Skip to content

Allow scheduling of CF completion in parallel#277

Open
fxbonnet wants to merge 3 commits into
graphql-java:masterfrom
fxbonnet:allow-scheduling-of-future-completions-in-parallel
Open

Allow scheduling of CF completion in parallel#277
fxbonnet wants to merge 3 commits into
graphql-java:masterfrom
fxbonnet:allow-scheduling-of-future-completions-in-parallel

Conversation

@fxbonnet
Copy link
Copy Markdown

When a batch loading function returns, we currently complete all CompletableFutures sequentially on the calling thread. Since CompletableFuture.complete(...) also executes any continuations attached via thenApply(...) or thenCompose(...), large batches can significantly delay the return of dispatch().

This PR introduces an option to execute these continuations in parallel instead.

clearCacheKeys.add(key);
}
} else {
future.complete(value);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This misses the Try path of completion

@andimarek
Copy link
Copy Markdown
Member

@fxbonnet I am not sure about this PR: with the new hook introduced by @bbakerman in #275 you can already offload every completion onto a new thread and hence it will run in parallel if you choose todo so. I have problems seeing how this approach is different/better.

@fxbonnet fxbonnet force-pushed the allow-scheduling-of-future-completions-in-parallel branch from dd2af99 to e04c76d Compare May 18, 2026 22:29
@fxbonnet
Copy link
Copy Markdown
Author

@andimarek I created this PR after my comment on @bbakerman 's PR #275 (comment)
The PR allowed to offload the completions to a separate thread, but in case the batch load contains multiple keys, the completions for each key would still happen sequentially.
For clarity I just rebased this PR onto @bbakerman 's branch. The main change is that instead of passing a single Runnable to scheduleCompletion, we would pass a List<Runnable> (one for each key), giving the option to execute them in parallel.

}

List<K> clearCacheKeys = new ArrayList<>();
Collection<K> clearCacheKeys = new ConcurrentLinkedQueue<>();
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clearCacheKeys may now be accessed concurrently by multiple thread if we execute the Runnables in parallel.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call - price of side effects outside a thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants