-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Pitch: V7 chain-specific default dispatchers #1023
Comments
People have asked this before (being able to change the default queue for a specific chain) and my argument generally was you return promises all over the place, and in use of such returns you wouldn't know what dispatcher you were getting which could easily lead to bugs. Like the argument still stands, but perhaps we should just make this a thing the user has to be aware of, that is, you need to reset the dispatcher before you return or document the fact that the dispatcher is not the default? Certainly we’ve all wanted this feature from time to time. Though PMKv6 distinguishing between in-between closures and Side note: conf.Q is meant to be one-time-set app-wide; it used to be set with a |
Yes, that seems like a real issue and a potential concern. I agree that the docs should advise strongly against returning promises that have a default dispatcher unless there's a specific reason to do that. I suspect that a predominant source of problems is likely to be that people set a default dispatcher for notational convenience and then send their promises elsewhere without consciously realizing that they're imposing a dispatcher on the recipient. It's obvious if you even think about it, but it's the kind of cleanup that's easy to overlook. I was thinking about what the various failure modes might be and trying to pick out the ones that are most worth worrying about. They're not all equally bad. For example, if you use the default dispatchers and someone puts you on a different thread, and then you try to do main-thread UI stuff, that's usually self-evident pretty quickly because of warning messages or crashes. That seems fine. And similarly, if you're using a nonstandard global default queue, that's probably a pretty good sign that you understand the dispatching system and can be left alone. The really bad failure mode, I think, is when you believe your own code is all main-threaded, but parts actually end up running on two or more threads. So it almost always works, but every so often there's a completely unexplained crash. 😣 A few possible ways to be safer:
dispatchOn(.global()) {
firstly {
doTimeConsumingThing()
}. then { a in
doOtherTimeConsumingThing().map { (a, $0) }
}
}.done { ab in ... } Disadvantages: 1) You lose the ability to return promises with default dispatchers. 2) Ugly code with another layer of nesting to track. 3) |
In RxSwift, dispatching is an attribute of the chain. You switch dispatchers with a separate chain operator, e.g.
I believe ReactiveSwift works similarly, but I'm not as familiar with that library.
I love PromiseKit's simple convention of requiring an explicit dispatcher specification for each step in a chain unless you want to use the global default. It's direct and easily understood. I wouldn't want to change this system as the default.
Nevertheless, there's no reason why PromiseKit couldn't also, optionally, allow you to set a default dispatcher for a chain, and there are a couple of nice benefits that would come from this ability.
I'll mention some of the advantages below, but let me first clarify exactly what I'm talking about. Here's a straw-man proposal:
setDefaultMapDispatcher
andsetDefaultReturnDispatcher
methods (terrible placeholder names...) with a signature something likeAs usual, the returned promise is different from the original; it's a wrapper. Only subsequent chain elements added via the wrapper see the new default.
Any subsequent method uses the specified default dispatcher if nothing more specific is requested.
conf.Q
still exists but is consulted only if no chain-specific default has been set.Methods that return new promises silently propagate the default dispatcher to them.
Benefits
Clearer and more self-documenting than
conf.Q
. I would hazard a guess that some users initially assume thatconf.Q
is consulted at the point of actual dispatch. But in fact, it's consulted at chain construction time through the default argument mechanism. This is useful because it allows differentconf.Q
values to be set for different regions of code, but it's not very discoverable without experimentation.Chains don't affect each other. By contrast, there's only one
conf.Q
, and code has to take explicit precautions (save/restore) to avoid fighting over it.Code is cleaner without repeating
on:
for every clause.Purely additive: nothing changes unless you use the feature.
Facilitates debugging because you can easily set a temporary default for any portion of a chain. It's one line added or removed which affects no other code. With
conf.Q
, you have to add a variable assignment at the start of the chain, stop in the middle of the chain to setconf.Q
, then resume the chain from the variable.The big one: it allows functions that yield or propagate promises to specify (suggest, really) dispatching policy. Result types that aren't thread-safe can return themselves with dispatching set to a serial queue. For example, a routine that returns a
Promise<NSManagedObject>
might set the chain's default dispatcher to aCoreDataDispatcher
for the appropriateNSManagedObjectContext
so that the managed object can be freely manipulated with no additional code on the receiver's end. E.g.,Thoughts?
The text was updated successfully, but these errors were encountered: