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 performance of _get_zipimporters #2428

Merged

Conversation

correctmost
Copy link
Contributor

Description

_get_zipimporters can call isinstance millions of times when running pylint's import-error checker on a codebase like yt-dlp.

Checking for None first avoids the overhead of invoking isinstance.

Closes pylint-dev/pylint#9607.

Performance

I ran pylint with just the import-error checker on the yt-dlp codebase. With pylint 3.1.0 and astroid 3.1.0, linting takes ~38.2 seconds. With the fix applied, linting takes ~36.3 seconds.

Type of Changes

Type
🔨 Refactoring

_get_zipimporters can call isinstance millions of times when
running pylint's import-error checker on a codebase like yt-dlp.

Checking for None first avoids the overhead of invoking isinstance.

Closes pylint-dev/pylint#9607.
@Pierre-Sassoulas Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label May 11, 2024
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Great find !

@Pierre-Sassoulas
Copy link
Member

(pypy 3.8 and 3.10 are flaky, nothing to do on your end)

@DanielNoord DanielNoord enabled auto-merge (rebase) May 11, 2024 08:02
Copy link

codecov bot commented May 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.59%. Comparing base (0ccc2e2) to head (5cd184f).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2428      +/-   ##
==========================================
- Coverage   92.78%   92.59%   -0.19%     
==========================================
  Files          94       94              
  Lines       11098    11098              
==========================================
- Hits        10297    10276      -21     
- Misses        801      822      +21     
Flag Coverage Δ
linux 92.59% <100.00%> (ø)
pypy ?
windows ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
astroid/interpreter/_import/spec.py 97.47% <100.00%> (ø)

... and 5 files with indirect coverage changes

@DanielNoord DanielNoord merged commit 2c38c02 into pylint-dev:main May 11, 2024
21 checks passed
@correctmost correctmost deleted the cm/speed-up-zipimporter-check branch May 11, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E0401 (import-error) checks perform many isinstance calls when searching for zipimporters
3 participants