Restrict pylint parallelism to available RAM #2918
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hello @martinpitt! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-10-13 05:05:24 UTC |
pylint uses *lots* of RAM (more than 1 GiB per process), and the `jobs=0` default runs `nproc` parallel processes by default. This exceeds the available memory in many standard environments (such as m1.medium == 2 CPUs/4 GiB RAM, or m1.large == 4 CPUs/8 GiB RAM) and causes eternal hangs of the pylintrc test when one job process gets OOM-killed. Instead, set the parallelism depending on the amount of free memory -- one job for each 2 GB of available RAM. This is an underestimation, but let's not use up the entire system memory. Also still cap it to the number of available CPUs, just in case this runs on a system with lots of RAM and only few CPUs.
martinpitt
force-pushed
the
pylint-jobs
branch
from
8a3cf17
to
be12ca7
Compare
5 tasks
poncovka
approved these changes
Oct 13, 2020
| @@ -116,6 +117,23 @@ def run(self): | |||
| def _prepare_args(self): | |||
| args = [] | |||
|
|
|||
| # pylint uses a lot of memory and doesn't handle ENOMEM well, so set --jobs based on memory | |||
There was a problem hiding this comment.
Do you think it might be worth it to report the issue to pylint?
There was a problem hiding this comment.
pylint-dev/pylint#1495 has existed for years, but I'll report a more specific one. Possibly fixed by/related to pylint-dev/pylint#3869
VladimirSlavik
approved these changes
Oct 13, 2020
jkonecny12
approved these changes
Oct 13, 2020
|
I filed pylint-dev/pylint#3899 about the hang after OOM, and followed up to pylint-dev/pylint#1495 with a current reproducer about the memory leak. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pylint uses lots of RAM (more than 1 GiB per process), and the
jobs=0default runsnprocparallel processes by default. Thisexceeds the available memory in many standard environments (such as
m1.medium == 2 CPUs/4 GiB RAM, or m1.large == 4 CPUs/8 GiB RAM) and
causes eternal hangs of the pylintrc test when one job process gets
OOM-killed.
Instead, set the parallelism depending on the amount of free memory --
one job for each 2 GB of available RAM. This is an underestimation, but
let's not use up the entire system memory. Also still cap it to the
number of available CPUs, just in case this runs on a system with lots
of RAM and only few CPUs.