Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concurrent operation scheduler #3468

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jarreds
Copy link

@jarreds jarreds commented Jan 3, 2025

In the spirit of optimized batching, I've been playing around with a coroutine implementation in Golang that allows cooperative multitasking to batch IO. I can share more details on that as the experiment progresses.

This is the code I used to shim in a new "scheduler" for gqlgen. Creating as draft for now until it proves out that there is actually reason to have this code.

@coveralls
Copy link

Coverage Status

coverage: 73.832% (-0.08%) from 73.915%
when pulling 569c57b on jarreds:operation-scheduler
into 3fc5a53 on 99designs:master.

@StevenACoffman
Copy link
Collaborator

Interesting! I'm very curious to learn what you find out!

However, I would suggest that in the Dispatch you really need the scheduler bypass when you only have a single task. Having this case where it doesn't go through a scheduler or anything complicated is an important performance feature for a surprising number of use cases. Every time we've removed it, there's been a storm of complaints.

func (m *FieldSet) Dispatch(ctx context.Context) {
	if len(m.delayed) == 1 {
		// only one concurrent task, no need to spawn a goroutine or deal create waitgroups

@jarreds
Copy link
Author

jarreds commented Jan 4, 2025

Noted! FWIW I did put a special case in the default goroutine scheduler to run len one tasks (w/o wait groups) and the last f "on-thread:" https://github.com/99designs/gqlgen/pull/3468/files#diff-3a919dfc5028540a932a129e10b6e9cb749a1fd70be686dc7471ff440d823086R37-R40

So it should launch the minimal number of goroutines like it did before.

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