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

Move the parallel execution code into Runner #936

Closed
AWhetter opened this issue Jun 10, 2016 · 4 comments
Closed

Move the parallel execution code into Runner #936

AWhetter opened this issue Jun 10, 2016 · 4 comments
Assignees

Comments

@AWhetter
Copy link
Contributor

Currently the code to deal with parallel execution lives inside the PyLinter class. This means that a PyLinter ends up running PyLinters when executing in parallel (via ChildLinters). It also means that a new PyLinter is created every time a file/module is read, and the config object has to be loaded into the new PyLinter instance.

We could eliminate this overhead by getting the Runner to launch threads, have a single PyLinter object per thread, and distribute the work to those threads.

This will also eliminate the need to change the jobs option in the config when passing it to child PyLinters.

@AWhetter AWhetter added Help wanted 🙏 Outside help would be appreciated, good for new contributors task labels Jun 10, 2016
@AWhetter AWhetter self-assigned this Jun 10, 2016
@AWhetter AWhetter removed the Help wanted 🙏 Outside help would be appreciated, good for new contributors label Jun 10, 2016
@The-Compiler
Copy link
Contributor

It uses subprocessing currently if I remember correctly - is that also your proposal for this change? Because threads will probably not work out due to the python GIL.

@AWhetter
Copy link
Contributor Author

My proposal is to move the parallel code out of PyLinter, scrap the ChildLinter, and move all of that code into Run. Then Run will spawn a single PyLinter per thread.

It's using multiprocessing rather than subprocessing at the moment. I don't want to change what threading library we use, or change the task queue technique we use at the moment.

@PCManticore
Copy link
Contributor

@AWhetter let's use process instead of thread, so we won't confuse people with what we plan to change.

Sounds like a good idea, but some things might need to be changed. For instance, the configuration needs to be done before starting the jobs, expand_files might need to move and each process needs to take care of loading the necessary plugins (this is currently done before it, through load_plugin_modules).

While we are it, can we rename Run into something else?

@AWhetter
Copy link
Contributor Author

AWhetter commented Jul 2, 2016

Implemented via #955

@AWhetter AWhetter closed this as completed Jul 2, 2016
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

3 participants