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

Allow parallel execution of mutation non-top level fields #2040

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

kyri-petrou
Copy link
Collaborator

Closes #2035

PS: I didn't find anything in the spec that specifies that subscriptions should be executed sequentially. @ghostdogpr @paulpdaniels do you happen to know how come and the execution mode for subscriptions was forced to Sequential?

@paulpdaniels
Copy link
Collaborator

I don't think it makes a difference for subscriptions because we just produce a response value that contains all the streams

case QueryExecution.Sequential => ZQuery.foreach(in)(as)
case QueryExecution.Parallel => ZQuery.foreachPar(in)(as)
case QueryExecution.Batched => ZQuery.foreachBatched(in)(as)
executionMode match {
Copy link
Owner

Choose a reason for hiding this comment

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

Does that really make a difference to use ints? Code would be cleaner without 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pattern matching on Ints in increments compiles to a tableswitch, which has O(1) performance regardless of the number of elements in the pattern matching. Plus I do think that equality on ints is less complex than on objects since there's no need for type-checking.

Having said that, we only have 3 elements, and this code is probably a millionth of the overall execution complexity so I'll just go back to making it more readable 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Made the change. I placed Sequential last now though since I find it very hard to believe anyone would ever want to use that.. right?

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks 😄

@kyri-petrou kyri-petrou merged commit 6cbc226 into series/2.x Dec 15, 2023
10 checks passed
@kyri-petrou kyri-petrou deleted the parallel-mutation-execution branch December 15, 2023 03:36
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.

Allow executing mutations / subscriptions in parallel / batched mode
3 participants