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

Nice way to run on specific queues #1

Closed
ide opened this issue Apr 22, 2014 · 22 comments
Closed

Nice way to run on specific queues #1

ide opened this issue Apr 22, 2014 · 22 comments

Comments

@ide
Copy link

ide commented Apr 22, 2014

One of the things that Bolts does pretty well is let you choose a queue to run a block on, while this is manual with PromiseKit. I think it'd be useful to be able to write:

dispatch_queue_t queue = ...;
dispatch_promise(queue, ^{
    return [self makeValueAsync];
}).then(dispatch_get_main_queue(), ^(id value) {
    [self.view displayValue:value];
    return [self logDisplayInfo:self.view];
}).then(^{
    // No queue specified: synchronously run on the same queue immediately after
    // logDisplayInfo's promise is resolved
});

Also Bolts has an "executor" abstraction that runs blocks. Executors might be more general than needed but the default executors are useful to have:

  • dispatch_async to the main queue
  • dispatch_async to the default-pri background queue
  • run synchronously on the current queue where the previous task finished
@mxcl
Copy link
Owner

mxcl commented Apr 22, 2014

Yes, I'll add the choose-a-queue variant, and probably add an NSOperationQueue category too.

For other kinds of "executor" I'll wait for people to propose additions or use cases.

@ide
Copy link
Author

ide commented Apr 22, 2014

Wonderful!

mxcl added a commit that referenced this issue Apr 22, 2014
@mxcl
Copy link
Owner

mxcl commented Apr 22, 2014

I started to consider the NSOperation variant and wasn't sure if it should be a category on NSOperationQueue or a subclass of NSOperation and what should then happen with then (ie. should the then's be operations also? If so, the complexity increases).

But as you can see above, I committed dispatch_promise_on.

@ide
Copy link
Author

ide commented Apr 23, 2014

A category of NSOperationQueue seems more flexible than subclassing NSOperation e.g. it accomodates projects that already subclass NSOperation. From my experience using Bolts, one of the most important features was being able to choose the dispatch queue / NSOperationQueue to run a block so being able to do that in PromiseKit would be good.

For then, I think it's simplest to always return a Promise and later add a convenience method to convert Promises to NSOperations.

@mxcl
Copy link
Owner

mxcl commented Apr 25, 2014

As an additional note, I will make it possible for then to specify a queue (like your example code shows) as part of this ticket.

@maximkhatskevich
Copy link

"I will make it possible for then to specify a queue" -->> Max, tell me please, when do you think this feature will be ready and available via CocoaPods?

@mxcl
Copy link
Owner

mxcl commented May 10, 2014

As soon as I can figure out how to do it elegantly. I can't really estimate that well, but I really would like to get this in ASAP.

@maximkhatskevich
Copy link

Okay, thanks. Look forward to it. In the mean time, does current implementation execute each then block in chain on Main queue?

For example:

dispatch_promise(^{
    return md5(email); // main queue???
}).then(^(NSString *md5){
    return [NSURLConnection GET:@"http://gravatar.com/%@", md5];  // main queue???
}).then(^(UIImage *gravatarImage){
    self.imageView.image = gravatarImage;  // main queue???
});

Explain please how can I know on which thread/queue a block in the chain would be executed?

@mxcl
Copy link
Owner

mxcl commented May 10, 2014

dispatch_promise is documented to run on a background queue. It thens onto the main queue.

There is also dispatch_promise_on if you need a specific queue. Also documented.

There is currently no guarantees about the queue a then runs on, it depends on the implementation of the Promise, it's just currently they all then onto main.

Thus to achieve your current goals you can do this for now:

somepromise.then(^{
    return dispatch_promise(^{
        //
    });
}).then(^{
    return dispatch_promise(^{
        //
    });
});

Not pretty, but works for now. Naturally this will mean the main queue is always used as an intermediate, but that should be fine under most circumstances. Just not ideal.

@maximkhatskevich
Copy link

Okay, thank you very much for the explanation!

@yvbeek
Copy link

yvbeek commented May 12, 2014

Perhaps this notation would work?

dispatch_queue_t myQueue = dispatch_queue_create(...);

dispatch_promise(^{
    return md5(email);
}).thenOn(myQueue, ^(NSString *md5){
    return [NSURLConnection GET:@"http://gravatar.com/%@", md5];
}).then(^(UIImage *gravatarImage){
    self.imageView.image = gravatarImage;
});

By the way, nice design on the block methods!

@mxcl
Copy link
Owner

mxcl commented May 12, 2014

@Zyphrax yes, I like that.

@maximkhatskevich
Copy link

@mxcl I would suggest to allow set a queue (lets name it defaultQueue) where to process all the blocks by default at promise constructor and allow specify queue as a parameter to change target queue for a particular "then"/block. Also use dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0) (system background concurrent queue) by default (I mean use it if no defaultQueue specified explicitly at promise creation time). I think this would be the most flexible way to implement it, because you give full control on where (on which queue) each block is being processed.

For most cases you can specify needed queue only twice (at creation time and at last block) like this:

dispatch_queue_t myQueue = dispatch_queue_create(...); // custom queue

dispatch_promise(myQueue, ^{
    return md5(email); // myQueue
}).thenOn(^(NSString *md5){
    return [NSURLConnection GET:@"http://gravatar.com/%@", md5]; // myQueue
}).then(dispatch_get_main_queue(), ^(UIImage *gravatarImage){
    self.imageView.image = gravatarImage; // dispatch_get_main_queue(),
});

... or even once (if you it is okay to perform everything on DEFAULT concurrent system queue except last block where you set explicitly MAIN queue) like this:

dispatch_promise(^{
    return md5(email); // default queue
}).thenOn(^(NSString *md5){
    return [NSURLConnection GET:@"http://gravatar.com/%@", md5]; // default queue
}).then(dispatch_get_main_queue(), ^(UIImage *gravatarImage){
    self.imageView.image = gravatarImage; // dispatch_get_main_queue(),
});

@yvbeek
Copy link

yvbeek commented May 12, 2014

I think the solution should cover at least these cases, in a clear and simple notation:

  1. You start a task on a concurrent queue and end up on the main (UI) queue.
  2. You start a task on a serial queue and end up on the main queue
  3. You do multiple concurrent actions and bring them together to a serial queue or the main queue

For option 1: PromiseKit could create the concurrent queue for us, priority argument would be nice.

For option 2: PromiseKit should be fed a queue argument, the serial queue is likely used in other parts of the code.

For option 3: This is probably were support for NSOperations is necessary to tie actions together.

Most people will probably need option 1 in their apps, which is in line with the current implementation.

The current implementation however is somewhat vague in terms of which queue is being used.

Perhaps .then should always continue on the same queue and simply add a .thenOn (for specific queue) and .thenOnMain or .thenOnUI (calls thenOn with main queue).

@yvbeek
Copy link

yvbeek commented May 12, 2014

This would mean that all your current examples would have to change the last .then to .thenOnMain.

(I'm on my phone at the moment, otherwise I would have created a code example).

@mxcl
Copy link
Owner

mxcl commented May 12, 2014

Then has to be mainQueue. Promises are cross-library solutions, and could come from any source. Their behavior has to be consistent, or their utility is reduced.

@yvbeek
Copy link

yvbeek commented May 13, 2014

Ok, that makes sense.

Perhaps use a different method to continue on the same queue (something like .continue)? Or wouldn't that be necessary because you could simply combine it with the first block?

@mxcl
Copy link
Owner

mxcl commented May 13, 2014

I'm not sure of an elegant solution. But for me, usually I only need one then block in any particular chain to be in a background queue, so continue isn't necessary. Maybe it's just the way I code. Perhaps you can provide an example where something like continue would make your code more elegant?

I try to resist new methods without compelling justification. Especially now, when the library is new, and how it will come to be used is not entirely defined.

@yvbeek
Copy link

yvbeek commented May 13, 2014

In my app I have the following case:

  1. Display HUD to user
  2. Upload logs to server
  3. When 2 is done, hide HUD and upload data dump to server
  4. When 3 is done, perform some internal clean-up (remove temp files etc)

So basically we want to upload the logs and data dump in the background, but during the logs upload we want to show a HUD to the user. The HUD needs to be controlled by the main queue, the two uploads on a serial or concurrent queue.

How would you solve that with PromiseKit?
I'm thinking: upload-log (background) => then => hide-hud (main) => then => upload-data-dump (background) => then => clean-up (background)

@andrebraga
Copy link

Would you find the syntax

.foreground() and .background(queueOrNull)

affecting subsequent calls to .then() to be too offensive?

Maybe make it so that only the next call to .then() runs on the designated queue?

Perhaps .background() could be chained to a call to .then() and only affect the immediately previous link? Would take some decent wrapping magic to achieve.

Anyway, that's just a quick snack for thought. :)

@mxcl
Copy link
Owner

mxcl commented May 19, 2014

@andrebraga the problem here is that if the promise crosses a library boundary you (the user) have no way of knowing what queue the then will execute on. then must be consistent.

@mxcl mxcl closed this as completed in dc87284 May 19, 2014
@yvbeek
Copy link

yvbeek commented May 19, 2014

Looks good 👍

mxcl pushed a commit that referenced this issue Oct 14, 2016
Enabled the skip install option, was causing app archive builds to work poorly.
@hamada147 hamada147 mentioned this issue Dec 11, 2017
mxcl pushed a commit that referenced this issue Aug 27, 2023
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

5 participants