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

DispatchQueues should be generalized #878

Closed
GarthSnyder opened this issue Jun 15, 2018 · 6 comments
Closed

DispatchQueues should be generalized #878

GarthSnyder opened this issue Jun 15, 2018 · 6 comments

Comments

@GarthSnyder
Copy link
Collaborator

I've been converting a project from RxSwift to PromiseKit, and it's going really well. I know these aren't regarded as competitive substitutes, but it has been a bit surprising to see how much cleaner some of the code gets.

The one thing that I'm feeling some regret about letting go of is RxSwift's Scheduler abstraction. Not because it's the world's most elegant API (though it's fine), but because it introduces a layer of abstraction between the Foundation-level facilities and the kit-level facilities. This separation is important because you can't actually subclass DispatchQueue: it's a C language system that's intimately intertwined with the compiler and runtime. If you want different dispatch behavior in PromiseKit, you're pretty much stuck, as far as I can tell. (Good thing it's easy to intermix manual GCD blocks with PromiseKit-managed blocks!)

I'll cite some specific use cases below, but just to get to the point: it would be nice if PromiseKit's on arguments accepted something like a DispatchProtocol object instead of an object-type-specific DispatchQueue. It's fine if the protocol mirrors the existing DispatchQueue API - I just want to be able to roll my own.

It looks like it would be a pretty straightforward change. I'd send it in as PR, except that I can imagine some strong potential objections to the basic idea. So I thought it would be better to get a temperature reading first. The main drawback is that DispatchProtocol becomes visible in the public API. I imagine most people's initial reaction would be something like "WTF is a DispatchProtocol?" Calling it something like DispatchQueueProtocol would perhaps clarify, but that's an awfully long name. Something like Scheduler is clear and succinct, but it obscures the fact that these objects really are just DispatchQueues in 99% of cases.

I have three use cases in the current project. One is a concurrency-limited queue. You can submit as many blocks as you like, but only N will be allowed to execute concurrently. The rest have to wait until a slot opens up. This is great for cases where you need to, e.g., read 100 XML feeds, but you know that URLSession will start acting up if you just fire them all off at once. At the same time, it's perfectly reasonable to queue up 100 blocks in a DispatchQueue, so you can get away with no queueing code at all outside of a generic concurrency-limited queue implementation.

The second case is rate-limited scheduling, where one of the external APIs limits the number of network requests that can be made in a given period of time. If you go over the limit, you get a significant punishment such as a 5-minute timeout, so it's worthwhile to obey the limit stringently. It's really easy to implement this in terms of DispatchQueues, but again, you need some way to interpose your own layer.

The last case is plain old Core Data. If I understand the architecture correctly, perform and performAndWait are just thin wrappers around dispatch queues. Nevertheless, they are the designated interface and you are supposed to use them instead of trying to get your hands on the underlying DispatchQueue. It would be really nice to be able to write, e.g., done(on: myCoreDataMOC) {...} and just have it use perform* behind the scenes.

Thoughts?

@mxcl
Copy link
Owner

mxcl commented Jun 15, 2018

I'm cool, and agree it would be useful, but there are a few conditions:

  1. We cannot break the public API, this includes that, for example, this must continue to work:
    foo.then(on: .main)
  2. It must not create ambiguity in use, we worked hard to reduce ambiguity for PMK6

We could put this in PMK 7 though. Although I'd still want on(.main) etc. to work.

@GarthSnyder
Copy link
Collaborator Author

Great! I'll see what I can do.

@mxcl
Copy link
Owner

mxcl commented Feb 12, 2019

We have a test breakage in CI for Linux, any ideas? https://travis-ci.org/mxcl/PromiseKit/builds/492338166

@GarthSnyder
Copy link
Collaborator Author

Looking at it now...

@GarthSnyder
Copy link
Collaborator Author

In the latest Linux library, DispatchQueue.main is evidently no longer a static object. You get a new instance each time, or perhaps it's a value type now. Odd, because you can still access the same queue-specific data through any of the instances...

The library code doesn't depend on this, just the testing code. PR incoming.

@mxcl
Copy link
Owner

mxcl commented Feb 13, 2019

Merged into v7.

@mxcl mxcl closed this as completed Feb 13, 2019
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

No branches or pull requests

2 participants