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

Parallelize hook runs #144

Closed
sds opened this issue Mar 25, 2015 · 2 comments
Closed

Parallelize hook runs #144

sds opened this issue Mar 25, 2015 · 2 comments
Milestone

Comments

@sds
Copy link
Owner

sds commented Mar 25, 2015

When executing hooks, it would be significantly faster to run hooks in parallel rather than serially.

Benefits

  • Faster hook runs that scale with the number of processors on a machine

Costs

  • The HookRunner class will need some serious rethinking to support the collection of hook statuses as they are completed in parallel
  • This will change overcommit to be a multi-threaded application, so interrupt handling (and signal handlers) will need to handle cases gracefully. Testing this will be difficult.
  • If Allow hooks to depend on other hooks #143 is implemented, parallelism is potentially hindered depending on the dependency graph. This also introduces complexity as we can no longer just run hooks in topological sort order, but would need to trigger dependent hooks to run once all their dependencies passed.

Questions

  • What happens if a hook generates files during its run?
    This is a potentially big issue. Since we're now running hooks in parallel the temporary files generated by some hooks could potentially affect other hook runs. We may have to support a parallelize option which can be set to false to force serial execution in extreme cases
  • What happens if a hook runs git commands?
    Similar to the file issue above, some git commands require exclusive access to the files in the .git directory during their run. We'll likely need a git helper (similar to execute) that serializes requests so hooks don't accidentally trample on each other

Feel free to leave comments and ask questions in this issue.

@Bertg
Copy link
Contributor

Bertg commented Apr 17, 2015

Yeah. I'm all for this :) 👍

@sds
Copy link
Owner Author

sds commented Feb 20, 2016

This was implemented in #290, and will be released in Overcommit 0.32.0 once it is ready.

@sds sds closed this as completed Feb 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants