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

Improve multitask performance #273

Merged
merged 1 commit into from
Nov 5, 2018
Merged

Improve multitask performance #273

merged 1 commit into from
Nov 5, 2018

Conversation

jsm
Copy link
Contributor

@jsm jsm commented Sep 28, 2018

Thread context switches are expensive, iterating through promises in reverse minimizes them.

See my blog post about it here: https://medium.com/@sanmiguelje/optimizing-rake-multitask-fece36a16c8f

@colby-swandale
Copy link
Member

@hsbt is there anything more that this PR needs to be merged?

@yuki24
Copy link
Member

yuki24 commented Nov 5, 2018

Interesting!

@hsbt anything blocking us from merging this?

@@ -248,7 +248,7 @@ def invoke_prerequisites_concurrently(task_args, invocation_chain)# :nodoc:
r.invoke_with_call_chain(prereq_args, invocation_chain)
end
end
futures.each(&:value)
futures.reverse_each(&:value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsm Can you add comment for performace reason about reverse_each ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hsbt Updated, let me know if you think I need to add more detail.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 97.751% when pulling 137e3f7 on jsm:master into b5f633a on ruby:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 97.751% when pulling 137e3f7 on jsm:master into b5f633a on ruby:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 97.751% when pulling 137e3f7 on jsm:master into b5f633a on ruby:master.

@hsbt
Copy link
Member

hsbt commented Nov 5, 2018

@jsm Thanks!

@hsbt hsbt merged commit f0c7528 into ruby:master Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants