Skip to content

Commit d77e1c2

Browse files
authored
Fix performance regression caused by #5465: non-daemon part (#5544)
The problem with the initial fix in #5465 is that it assumes that all modules that are in `st.suppressed` that now exist in file system are newly added, while in fact they can be not new if one uses `follow_imports = skip`. My initial guess was that this causes a lot of files become stale. But it actually turns out that the staleness is determined correctly (all the tests I added actually pass on master). However, we still may parse a lot of modules unnecessarily. This PR fixes the performance regression by: not treating modules as newly added if they were added to `st.suppressed` because of `follow_imports = skip` There still some overhead remains -- we need to clone options for module to understand why it got into suppressed dependencies. This can be improved by having to separate suppressed lists, but I think this win will be quite minor and it is not worth the complexity.
1 parent 480b153 commit d77e1c2

File tree

2 files changed

+96
-10
lines changed

2 files changed

+96
-10
lines changed

mypy/build.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1817,13 +1817,7 @@ def __init__(self,
18171817
# suppressed dependencies. Therefore, when the package with module is added,
18181818
# we need to re-calculate dependencies.
18191819
# NOTE: see comment below for why we skip this in fine grained mode.
1820-
new_packages = False
1821-
for dep in self.suppressed:
1822-
path = manager.find_module_cache.find_module(dep, manager.search_paths,
1823-
manager.options.python_executable)
1824-
if path and '__init__.py' in path:
1825-
new_packages = True
1826-
if new_packages:
1820+
if exist_added_packages(self.suppressed, manager, self.options):
18271821
self.parse_file() # This is safe because the cache is anyway stale.
18281822
self.compute_dependencies()
18291823
else:
@@ -2369,6 +2363,30 @@ def find_module_and_diagnose(manager: BuildManager,
23692363
raise CompileError(["mypy: can't find module '%s'" % id])
23702364

23712365

2366+
def exist_added_packages(suppressed: List[str],
2367+
manager: BuildManager, options: Options) -> bool:
2368+
"""Find if there are any newly added packages that were previously suppressed.
2369+
2370+
Exclude everything not in build for follow-imports=skip.
2371+
"""
2372+
for dep in suppressed:
2373+
if dep in manager.source_set.source_modules:
2374+
# We don't need to add any special logic for this. If a module
2375+
# is added to build, importers will be invalidated by normal mechanism.
2376+
continue
2377+
path = find_module_simple(dep, manager)
2378+
if not path:
2379+
continue
2380+
if (options.follow_imports == 'skip' and
2381+
(not path.endswith('.pyi') or options.follow_imports_for_stubs)):
2382+
continue
2383+
if '__init__.py' in path:
2384+
# It is better to have a bit lenient test, this will only slightly reduce
2385+
# performance, while having a too strict test may affect correctness.
2386+
return True
2387+
return False
2388+
2389+
23722390
def find_module_simple(id: str, manager: BuildManager) -> Optional[str]:
23732391
"""Find a filesystem path for module `id` or `None` if not found."""
23742392
return manager.find_module_cache.find_module(id, manager.search_paths,
@@ -2661,9 +2679,6 @@ def load_graph(sources: List[BuildSource], manager: BuildManager,
26612679
# comment about this in `State.__init__()`.
26622680
added = []
26632681
for dep in st.ancestors + dependencies + st.suppressed:
2664-
# We don't want to recheck imports marked with '# type: ignore'
2665-
# so we ignore any suppressed module not explicitly re-included
2666-
# from the command line.
26672682
ignored = dep in st.suppressed and dep not in entry_points
26682683
if ignored and dep not in added:
26692684
manager.missing_modules.add(dep)

test-data/unit/check-incremental.test

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4884,3 +4884,74 @@ def f(x: str) -> None: pass
48844884
[out]
48854885
[out2]
48864886
main:2: error: Argument 1 to "f" has incompatible type "int"; expected "str"
4887+
4888+
[case testFollowImportSkipNotInvalidatedOnPresent]
4889+
# flags: --follow-imports=skip
4890+
# cmd: mypy -m main
4891+
[file main.py]
4892+
import other
4893+
[file other.py]
4894+
x = 1
4895+
[file other.py.2]
4896+
x = 'hi'
4897+
[stale]
4898+
[rechecked]
4899+
4900+
[case testFollowImportSkipNotInvalidatedOnPresentPackage]
4901+
# flags: --follow-imports=skip
4902+
# cmd: mypy -m main
4903+
[file main.py]
4904+
import other
4905+
[file other/__init__.py]
4906+
x = 1
4907+
[file other/__init__.py.2]
4908+
x = 'hi'
4909+
[stale]
4910+
[rechecked]
4911+
4912+
[case testFollowImportSkipNotInvalidatedOnAdded]
4913+
# flags: --follow-imports=skip --ignore-missing-imports
4914+
# cmd: mypy -m main
4915+
[file main.py]
4916+
import other
4917+
[file other.py.2]
4918+
x = 1
4919+
[stale]
4920+
[rechecked]
4921+
4922+
[case testFollowImportSkipInvalidatedOnAddedStub]
4923+
# flags: --follow-imports=skip --ignore-missing-imports
4924+
# cmd: mypy -m main
4925+
[file main.py]
4926+
import other
4927+
[file other.pyi.2]
4928+
x = 1
4929+
[stale main, other]
4930+
[rechecked main, other]
4931+
4932+
[case testFollowImportSkipNotInvalidatedOnAddedStubOnFollowForStubs]
4933+
# flags: --follow-imports=skip --ignore-missing-imports --config-file=tmp/mypy.ini
4934+
# cmd: mypy -m main
4935+
[file main.py]
4936+
import other
4937+
[file other.pyi.2]
4938+
x = 1
4939+
[file mypy.ini]
4940+
[[mypy]
4941+
follow_imports_for_stubs = True
4942+
[stale]
4943+
[rechecked]
4944+
4945+
[case testAddedSkippedStubsPackageFrom]
4946+
# flags: --follow-imports=skip --ignore-missing-imports
4947+
# cmd: mypy -m main
4948+
# cmd2: mypy -m main package package.missing
4949+
[file main.py]
4950+
from package import missing
4951+
missing.f(int())
4952+
[file package/__init__.py]
4953+
[file package/missing.py]
4954+
def f(x: str) -> None: pass
4955+
[out]
4956+
[out2]
4957+
tmp/main.py:2: error: Argument 1 to "f" has incompatible type "int"; expected "str"

0 commit comments

Comments
 (0)