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 queue, RunLoop.main and DispatchQueue.main, #133

Open
mycroftcanner opened this issue Nov 24, 2019 · 7 comments
Open

concurrent queue, RunLoop.main and DispatchQueue.main, #133

mycroftcanner opened this issue Nov 24, 2019 · 7 comments
Labels
enhancement New feature or request

Comments

@mycroftcanner
Copy link

There is an interesting discussion about the difference between RunLoop.main or DispatchQueue.main

RunLoop.main as a Scheduler ends up calling RunLoop.main.perform whereas DispatchQueue.main calls DispatchQueue.main.async to do work, for practical purposes they are nearly isomorphic. The only real differential is that the RunLoop call ends up being executed in a different spot in the RunLoop callouts whereas the DispatchQueue variant will perhaps execute immediately if optimizations in libdispatch kick in. In reality you should never really see a difference tween the two.

Philippe Hausler says:

RunLoop should be when you have a dedicated thread with a RunLoop running, DispatchQueue can be any queue scenario (and for the record please avoid running RunLoops in DispatchQueues, it causes some really gnarly resource usage...).

Philippe Hausler also mentioned:

Also it is worth noting that the DispatchQueue used as a scheduler must always be serial to adhere to the contracts of Combine's operators.

@mycroftcanner
Copy link
Author

Philippe Hausler:

So to clarify the commentary about serial queues only applies to the beta SDKs. It is something we are considering addressing systemically in the operators that take schedulers. That is a careful balance between the cost of locking versus the cost of correctness for exclusivity of streams: there are other prior art that are similar here: https://github.com/reactive-streams/reactive-streams-jvm/blob/v1.0.2/README.md#specification 8 specifically rule 1.03 requires the events to be thread safe.

Per the RunLoop those are currently scheduled in the common mode. I would be interested if there is actually a use case of having options to use a different mode. Please file a feedback/radar that is something that we should address if it has a serious use case.

@mycroftcanner
Copy link
Author

mycroftcanner commented Nov 24, 2019

Another thing I noticed tonight is that Publishers.CombineLatest* and the .combineLatest* operators will receive on the main thread unless you specify the scheduler again using receive(on:options:)

@heckj heckj added the enhancement New feature or request label Nov 24, 2019
@mycroftcanner
Copy link
Author

mycroftcanner commented Nov 25, 2019

@heckj I am using your book on a daily basis as some easily searchable reference (and I thank you heartedly for that. Happy to contribute or at least post things I find either confusing or missing as issue if that's okay with you.

@heckj
Copy link
Owner

heckj commented Nov 26, 2019

@mycroftcanner totally great by me! i love the feedback and external view of confusing points - both with my writing and how people are seeing combine (as an avenue to find better ways to explain what it does do)

@marktrobinson
Copy link

marktrobinson commented Jan 2, 2020

Philippe Hausler says:

So to follow up here, there are some changes incoming in the regards to the way downstream events are propagated. We are now able to satisfy the constraint of 1.03 even if the DispatchQueue is concurrent or the OperationQueue is not a restriction of maxConcurrentOperations of 1, or for that matter any valid scheduler being concurrent; we will always send serialized events on that requested scheduler for .receive(on:). The one remaining caveat that we deviate from the specification slightly is that upstream events such as cancel() and request(_:) in our world can happen concurrently. That being said we do handle them in a thread safe manner.
Source

So is this statement in the docs no longer true then?

From Docs:

When creating a DispatchQueue to use with Combine publishers on background threads, it is recommended that you use a regular serial queue rather than a concurrent queue to allow Combine to adhere to its contracts. That is - don’t create the queue with attributes: .concurrent.
Source

@heckj
Copy link
Owner

heckj commented Jan 2, 2020

@marktrobinson I've not found a way to test it in either direction, so I'm sticking with the "better safe than sorry" and using only serial queues myself. In my attempts to test it, I found no noticeable difference between concurrent or serial. Not knowing the internals I'm not sure that I really had any great insight on how to stress or strain the system in such a way that the constraints would potentially be violated.

That source message from Phillippe was from July 2019, but nothing about it was every discretely linked to a release or corresponding issue. Short answer is, I just really don't know.

@marktrobinson
Copy link

Yep fair! Thought i'd just raise the question, thanks for the reply @heckj 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants