Skip to content

SR-11854: Fix concurrent operations execution in OperationQueue #2937

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

Merged
merged 1 commit into from
Dec 18, 2020

Conversation

lxbndr
Copy link
Contributor

@lxbndr lxbndr commented Dec 6, 2020

To be able to run operations concurrently OperationQueue needs concurrent underlying DispatchQueue. This patch uses .concurrent attribute to create DispatchQueues with required behavior.

This matches with Darwin.

To be able to run operations concurrently OperationQueue needs
concurrent underlying DispatchQueue.

This matches with Darwin.
@lxbndr
Copy link
Contributor Author

lxbndr commented Dec 6, 2020

cc @compnerd @spevans @millenomi

@millenomi
Copy link
Contributor

cc @phausler — was this intended?

@spevans
Copy link
Contributor

spevans commented Dec 7, 2020

@swift-ci test

1 similar comment
@spevans
Copy link
Contributor

spevans commented Dec 8, 2020

@swift-ci test

@spevans
Copy link
Contributor

spevans commented Dec 10, 2020

Does this fix https://bugs.swift.org/browse/SR-11854 (OperationQueue not asynchronous) ?

@lxbndr
Copy link
Contributor Author

lxbndr commented Dec 10, 2020

Oh, yes, exactly this case.

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me, though it would be nice to have an additional test to validate the state of the dispatch queue if Foundation is built testable. That said, it would be beyond the scope of the particular fix, so I think that it is okay to do that as a follow-up.

@compnerd
Copy link
Member

CC: @tomerd - this is probably a good thing to grab for the upcoming release as well

@tomerd
Copy link

tomerd commented Dec 18, 2020

CC: @tomerd - this is probably a good thing to grab for the upcoming release as well

@compnerd sounds good but we need this merged to main first, then a PR against the 5.3 branch.

cc @millenomi @spevans

@spevans
Copy link
Contributor

spevans commented Dec 18, 2020

This will also need to go to 5.4 as well. I think #2930 may also need to be backported to 5.3 as well as its another OperationQueue fix

@tomerd
Copy link

tomerd commented Dec 18, 2020

This will also need to go to 5.4 as well. I think #2930 may also need to be backported to 5.3 as well as its another OperationQueue fix

sgtm, do you want to merge this to main and then @lxbndr could backport them into 5.3 and 5.4?

@spevans spevans merged commit d24feed into swiftlang:main Dec 18, 2020
@lxbndr lxbndr deleted the really-concurrent-queue branch December 19, 2020 12:14
@lxbndr lxbndr changed the title Fix concurrent operations execution in OperationQueue SR-11854: Fix concurrent operations execution in OperationQueue Jan 18, 2021
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.

5 participants