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

Avoid unnecessary call to ‘continueWithBlock:’ when a task is completed #57

Merged
merged 1 commit into from
May 12, 2015

Conversation

BrunoBerisso
Copy link

When a continuation block is executed with a BFExecutor the internal block that makes the actual call check if the continuation block returns a BFTask, if that is the case a call to continueWithBlock: is made and the completion source is updated accordingly to the result of the task.

This pull request add code to check if the returned task is complete and update the completion source without made an extra call to continueWithBlock:. This increase the performance on memory consumption and execution time. And also make sence :)

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@josephearl
Copy link

LGTM

@nlutsenko
Copy link
Member

Took a while to get to, so sorry...
Will merge in tomorrow. Feel free to fix that one nit in the meantime.

@nlutsenko nlutsenko self-assigned this May 11, 2015
nlutsenko added a commit that referenced this pull request May 12, 2015
Avoid unnecessary call to ‘continueWithBlock:’ when a task is completed
@nlutsenko nlutsenko merged commit bd84de7 into BoltsFramework:master May 12, 2015
@BrunoBerisso
Copy link
Author

Hey, sorry for the delay on the completed / isCompleted. Should i open another pull request for this?

I think i choose isComplete to avoid access the inner _completed ivar bypassing the resultTask lock. Maybe i should use [resultTask isCompleted] instead the dot syntax.

@nlutsenko
Copy link
Member

resultTask.completed will go through using isComplete as it's a property access. resultTask->_isCompleted will use an ivar :D

You are more than welcome to open another pull request to use resultTask.completed. 👍

@BrunoBerisso
Copy link
Author

Got it, i don't use getter - setters to much :P
Thanks for the explanation, the new pull request is open.

Thanks for your awesome work with Bolts.

@facebook-github-bot
Copy link
Contributor

@BrunoBerisso updated the pull request.

@BrunoBerisso
Copy link
Author

Not sure what I supposed to do here.

@nlutsenko
Copy link
Member

@BrunoBerisso, completely discard that message. Our GitHub bot went nuts a little bit yesterday.
We calmed him down and restrained him for now, so we don't get mass spam in future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants