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

Clear pylint's LRU caches when using --clear-cache-post-run #8361

Closed
bersbersbers opened this issue Feb 28, 2023 · 20 comments · Fixed by #8420, #8521 or #9093
Closed

Clear pylint's LRU caches when using --clear-cache-post-run #8361

bersbersbers opened this issue Feb 28, 2023 · 20 comments · Fixed by #8420, #8521 or #9093
Assignees
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@bersbersbers
Copy link

Bug description

Many times a day, a python.exe process launched by ms-python.pylint takes up several GB of memory, and is uneager to release it. This has been reported at microsoft/vscode-pylint#280 with fairly straightforward reproduction steps. This is the result for me:
image

microsoft/vscode-pylint#280 credibly (for me) claims this should be reported here, so that's what I do.

Configuration

Irrelevant, I believe

Command used

Used extension

Pylint output

Irrelevant

Expected behavior

Irrelevant

Pylint version

pylint 2.16.2
astroid 2.14.2
Python 3.10.10 (tags/v3.10.10:aad5f6a, Feb  7 2023, 17:20:36) [MSC v.1929 64 bit (AMD64)]

OS / Environment

Windows 10 21H2

Additional dependencies

No response

@bersbersbers bersbersbers added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Feb 28, 2023
@Pierre-Sassoulas
Copy link
Member

Reproduction code from the vscode issue: x = b"a" * 4_000_000_000 # 4 GB bytes object.

@Pierre-Sassoulas Pierre-Sassoulas added Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning Bug 🪲 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Feb 28, 2023
@DanielNoord
Copy link
Collaborator

I'm in favour of closing this. Creating an object with a size of 4GB will obviously cause large memory usage, the issue in VS Code shows how the cache can be fully cleared after each run.

@Pierre-Sassoulas
Copy link
Member

I think it depends, do you encounter the problem on 2023.2.0 / 2.16.1 @bersbersbers ? If the cache is not cleared like it should with the clear-cache-post-run option then it's something we need to fix or that vscode needs to fix.

@bersbersbers
Copy link
Author

bersbersbers commented Feb 28, 2023

@Pierre-Sassoulas I am experiencing this issue on 2023.2.0 with pylint 2.16.2 installed in my virtual environment - not sure how that should behave. Oh, by the way, I am using "pylint.importStrategy": "fromEnvironment".

@bersbersbers
Copy link
Author

bersbersbers commented Feb 28, 2023

  • I am seeing the same thing without "pylint.importStrategy": "fromEnvironment", which falls back to using 2.16.1.

  • Even with x = b"a" * 1_000_000_000 (1 GB), I see memory consumption of up to 6 GB (first issue) which is not coming down any more (second issue) even after closing all files

  • I am seeing similar issues (up to 4 GB of memory used) even without that large object, just by working with one (somewhat longer, 2000+ LOC) Python file where I have no idea why any single element should require a lot of memory, supporting the leak hypothesis.

  • I am not saying this is pylints fault, but since Large memory usage, memory leaks microsoft/vscode-pylint#280 said it's pylint, who am I to judge? ;)

  • If the cache is not cleared like it should with the clear-cache-post-run option then it's something we need to fix or that vscode needs to fix.

    I can at least report that that flag is being used:

    2023-02-28 11:32:30.750 [info] c:\Users\bers.pyenv-win-venv\envs\project_3.10\Scripts\python.exe -m pylint --reports=n --output-format=json --clear-cache-post-run=y --from-stdin c:\Code\project\project\bug2.py

@jacobtylerwalls
Copy link
Member

Let's keep this open for some investigation, with the understanding we may or may not identify a follow-up action. IIUC Python doesn't always release memory back to the OS even after manual garbage collection.

@2-5
Copy link

2-5 commented Feb 28, 2023

Creating an object with a size of 4GB will obviously cause large memory usage.

This is just a one-line minimal reproduction. I encounter this issue daily working with regular Python files and Jupyter notebooks in VS Code.

I constantly monitor how much memory Python processes use on my machine for other reasons, and I noticed this. I suspect the issue is much more widespread, but people don't notice it, in my case it leaks around 1 GB per hour of VS Code usage. The most I've seen is 6 GB, but I guess it depends on what code you're editing.

@Pierre-Sassoulas
Copy link
Member

Look like there's something wrong with the pylint server like / daemon. Maybe it's the way we handle cache possibly in astroid. This will require investigation, I suggest to turn it off and on again from time to time if server mode is worth it, or to try running it without server mode otherwise, while we investigate the issue.

@2-5
Copy link

2-5 commented Feb 28, 2023

IIUC Python doesn't always release memory back to the OS even after manual garbage collection.

I don't think this is the issue here. Python did indeed had this problem a long time ago (in the 2.x era), but it was mostly fixed. Deleting the object promptly releases the memory on the same machine I encounter this issue:

x = b"a" * 4_000_000_000
del x

@jacobtylerwalls jacobtylerwalls self-assigned this Mar 3, 2023
@jacobtylerwalls
Copy link
Member

@DanielNoord identified the reason astroid is leaking memory: there's a reference cycle from child nodes back to their parents. Probably a classic use case for weakrefs, but I don't know how much of a lift that would be. Maybe an astroid 3.0 thing?

Thanks for the report; closing as duplicate of pylint-dev/astroid#1780

@jacobtylerwalls jacobtylerwalls closed this as not planned Won't fix, can't repro, duplicate, stale Mar 4, 2023
@jacobtylerwalls jacobtylerwalls removed their assignment Mar 4, 2023
@jacobtylerwalls jacobtylerwalls added Astroid Related to astroid Duplicate 🐫 Duplicate of an already existing issue and removed Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning Astroid Related to astroid Duplicate 🐫 Duplicate of an already existing issue labels Mar 4, 2023
@jacobtylerwalls
Copy link
Member

Although there are astroid updates needed, there are also pylint updates I just identified, so I'll reopen this to track that work. Apologies for the churn.

We need to introduce a similar mechanism as astroid has for clearing the LRU caches for any calls to pylint with --clear-cache-post-run as the VS Code extension uses.

We can then call it here.

For the sample code from OP, the changes I'm suggesting together with pylint-dev/astroid#2043 vastly improved memory allocation.

@jacobtylerwalls jacobtylerwalls added this to the 2.17.0 milestone Mar 4, 2023
@jacobtylerwalls jacobtylerwalls added Good first issue Friendly and approachable by new contributors Needs PR This issue is accepted, sufficiently specified and now needs an implementation labels Mar 4, 2023
@jacobtylerwalls jacobtylerwalls changed the title Excessive memory use/leak Clear pylint's LRU caches when using --clear-cache-post-run Mar 4, 2023
@Pierre-Sassoulas Pierre-Sassoulas removed this from the 2.17.0 milestone Mar 7, 2023
@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.0.0 milestone Mar 7, 2023
@jacobtylerwalls jacobtylerwalls removed the Good first issue Friendly and approachable by new contributors label Mar 10, 2023
@jacobtylerwalls jacobtylerwalls self-assigned this Mar 10, 2023
@jacobtylerwalls jacobtylerwalls modified the milestones: 3.0.0, 2.17.1 Mar 10, 2023
@Pierre-Sassoulas
Copy link
Member

Seems like it's not fixed by what was put in 2.17.1 alone: microsoft/vscode-pylint#280 (comment)

@Pierre-Sassoulas Pierre-Sassoulas removed this from the 2.17.1 milestone Mar 22, 2023
@bersbersbers
Copy link
Author

Seems like it's not fixed by what was put in 2.17.1 alone: microsoft/vscode-pylint#280 (comment)

Yes - I was hoping that pylint-dev/astroid#1780 would still have some things left to be released, but I understand that pylint-dev/astroid#2043 should be in astroid 2.15.0 already, and that is what I have been testing with.

Each time I save a Python file kick off a pylint run via the VS Code extension, memory consumption grows by 1 GB (using xyzab = b"a" * 1_000_000_000):
image

And the extension clearly reports using 2.17.1/2.15.0:

2023-03-22 20:17:54.130 [info] c:\Users\bers\.pyenv-win-venv\envs\project_3.11\Scripts\python.exe -m pylint --version
2023-03-22 20:17:54.137 [info] CWD Linter: c:\code\project
2023-03-22 20:17:59.535 [info] 
pylint 2.17.1
astroid 2.15.0
Python 3.11.2 (tags/v3.11.2:878ead1, Feb  7 2023, 16:38:35) [MSC v.1934 64 bit (AMD64)]


2023-03-22 20:18:00.499 [info] Version info for linter running for c:\code\project:
pylint 2.17.1
astroid 2.15.0
Python 3.11.2 (tags/v3.11.2:878ead1, Feb  7 2023, 16:38:35) [MSC v.1934 64 bit (AMD64)]

2023-03-22 20:18:01.055 [info] SUPPORTED pylint>=2.12.2
FOUND pylint==2.17.1

2023-03-22 20:18:04.316 [info] [Trace - 8:18:04 PM] Sending request 'textDocument/codeAction - (1)'.
2023-03-22 20:18:05.665 [info] [Trace - 8:18:05 PM] Received notification 'window/logMessage'.
2023-03-22 20:18:05.665 [info] c:\Users\bers\.pyenv-win-venv\envs\project_3.11\Scripts\python.exe -m pylint --reports=n --output-format=json --from-stdin c:\Code\project\project\bug.py
...

@jacobtylerwalls
Copy link
Member

Yes - I was hoping that pylint-dev/astroid#1780 would still have some things left to be released,

Indeed, I wouldn't retest this until that's in a released version of astroid.

Seems like it's not fixed by what was put in 2.17.1 alone: microsoft/vscode-pylint#280 (comment)

I'd argue for closing this issue since we re-scoped it to "clear pylint's LRU caches when using --clear-cache-post-run", not fix all of astroid's leaks.

@bersbersbers
Copy link
Author

@jacobtylerwalls oh, are you saying that what you reported in pylint-dev/astroid#1780 (comment) goes beyond #2043, so there is stuff waiting to be released that might fix the most major issue (except for dicts)? That would be cool, but I am a bit worried by milestone 3.0 mentioned in pylint-dev/astroid#1780 (comment) ;)

@Pierre-Sassoulas
Copy link
Member

We have a lot to do for the pylint 3.0.0 release but most deprecation are already handled and we're going to release alphas and beta. We will release the astroid cache change soon after the work is completed in astorid. Regarding astroid 3.0 we can be a little less drastic with the deprecations because we're the main consumer of astroid (98% of astroid users, use astroid through pylint).

@jacobtylerwalls
Copy link
Member

jacobtylerwalls commented Mar 23, 2023

oh, are you saying that what you reported in pylint-dev/astroid#1780 (comment) goes beyond #2043, so there is stuff waiting to be released that might fix the most major issue (except for dicts)?

Thanks for encouraging me to look closer :-)

Looks like pylint-dev/astroid#1780 (comment) already is achieved in 2.15.0. EDIT: but since one reference is all that takes to leak, I don't know if the current state of things is enough to solve the bulk of your day-to-day issue.

What VS Code extension are you using? From this comment, I see 2023.2.0 is the minimum version.

From your log, this invocation is missing the --clear-cache-post-run flag:

2023-03-22 20:18:05.665 [info] c:\Users\bers\.pyenv-win-venv\envs\project_3.11\Scripts\python.exe -m pylint --reports=n --output-format=json --from-stdin c:\Code\project\project\bug.py

@bersbersbers
Copy link
Author

From your log, this invocation is missing the --clear-cache-post-run flag:

Aha! Yes, adding

    "pylint.args": [
        "--clear-cache-post-run",
    ],

fixes the issue. So it seems 2.17.1/2.15.0 has all that's needed, thank you :)

I am using the most recent release version of the extension (2023.4.0), same thing with the latest prerelease (v2023.5.10801010). I'll post more details on why the flag is missing in microsoft/vscode-pylint#280.

@DanielNoord
Copy link
Collaborator

@jacobtylerwalls Does that mean congratulations are in order? Well done on fixing this issue!

@Pierre-Sassoulas
Copy link
Member

That's amazing @jacobtylerwalls ! I has given up on memory leaks, but your success with it make me hopeful about astroid's future performances too 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
5 participants