From 20ba59365c391b3423b0e2e635336fb658c81754 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 15 Feb 2021 15:38:31 +0000 Subject: [PATCH 1/3] Speed up case sensitive is-file check in file system cache This speeds up resolving imports somewhat. Add fast paths to `isfile_case`. On Linux we skip all logic, since the file system is almost always case sensitive. Cache results of parent directories as well. Add tests. --- mypy/fscache.py | 60 +++++++++++++++++++--------- mypy/test/testfscache.py | 84 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 18 deletions(-) create mode 100644 mypy/test/testfscache.py diff --git a/mypy/fscache.py b/mypy/fscache.py index 368120ea904ea..ef6149338a0e1 100644 --- a/mypy/fscache.py +++ b/mypy/fscache.py @@ -30,6 +30,7 @@ import os import stat +import sys from typing import Dict, List, Set from mypy.util import hash_digest from mypy_extensions import mypyc_attr @@ -53,6 +54,7 @@ def flush(self) -> None: self.listdir_cache = {} # type: Dict[str, List[str]] self.listdir_error_cache = {} # type: Dict[str, OSError] self.isfile_case_cache = {} # type: Dict[str, bool] + self.exists_case_cache = {} # type: Dict[str, bool] self.read_cache = {} # type: Dict[str, bytes] self.read_error_cache = {} # type: Dict[str, Exception] self.hash_cache = {} # type: Dict[str, str] @@ -197,32 +199,54 @@ def isfile_case(self, path: str, prefix: str) -> bool: The caller must ensure that prefix is a valid file system prefix of path. """ + if sys.platform == "linux": + # Assume that the file system on Linux is case sensitive + return self.isfile(path) + if not self.isfile(path): + # Fast path + return False if path in self.isfile_case_cache: return self.isfile_case_cache[path] head, tail = os.path.split(path) if not tail: + self.isfile_case_cache[path] = False + return False + try: + names = self.listdir(head) + # This allows one to check file name case sensitively in + # case-insensitive filesystems. + res = tail in names + except OSError: res = False - else: - try: - names = self.listdir(head) - # This allows one to check file name case sensitively in - # case-insensitive filesystems. - res = tail in names and self.isfile(path) - except OSError: - res = False - - # Also check the other path components in case sensitive way. - head, dir = os.path.split(head) - while res and head and dir and head.startswith(prefix): - try: - res = dir in self.listdir(head) - except OSError: - res = False - head, dir = os.path.split(head) - + if res: + # Also recursively check the other path components in case sensitive way. + res = self._exists_case(head, prefix) self.isfile_case_cache[path] = res return res + def _exists_case(self, path: str, prefix: str) -> bool: + """Helper to check path components in case sensitive fashion, up to prefix.""" + if path in self.exists_case_cache: + return self.exists_case_cache[path] + head, tail = os.path.split(path) + if not head.startswith(prefix): + # Only perform the check for paths under prefix. + self.exists_case_cache[path] = True + return True + assert tail + try: + names = self.listdir(head) + # This allows one to check file name case sensitively in + # case-insensitive filesystems. + res = tail in names + except OSError: + res = False + if res: + # Also recursively check other path components. + res = self._exists_case(head, prefix) + self.exists_case_cache[path] = res + return res + def isdir(self, path: str) -> bool: try: st = self.stat(path) diff --git a/mypy/test/testfscache.py b/mypy/test/testfscache.py new file mode 100644 index 0000000000000..86c808ccaccbf --- /dev/null +++ b/mypy/test/testfscache.py @@ -0,0 +1,84 @@ +"""Unit tests for file system cache.""" + +import os +import shutil +import sys +import tempfile +import unittest +from typing import Optional + +from mypy.fscache import FileSystemCache + + +class TestFileSystemCache(unittest.TestCase): + def setUp(self) -> None: + self.tempdir = tempfile.mkdtemp() + self.oldcwd = os.getcwd() + os.chdir(self.tempdir) + self.fscache = FileSystemCache() + + def tearDown(self) -> None: + os.chdir(self.oldcwd) + shutil.rmtree(self.tempdir) + + def test_isfile_case_1(self) -> None: + self.make_file('bar.py') + self.make_file('pkg/sub_package/__init__.py') + self.make_file('pkg/sub_package/foo.py') + # Run twice to test bost cached and non-cached code paths. + for i in range(2): + assert self.isfile_case('bar.py') + assert self.isfile_case('pkg/sub_package/__init__.py') + assert self.isfile_case('pkg/sub_package/foo.py') + assert not self.isfile_case('non_existent.py') + assert not self.isfile_case('pkg/non_existent.py') + assert not self.isfile_case('pkg/') + assert not self.isfile_case('bar.py/') + for i in range(2): + assert not self.isfile_case('Bar.py') + assert not self.isfile_case('pkg/sub_package/__init__.PY') + assert not self.isfile_case('pkg/Sub_Package/foo.py') + assert not self.isfile_case('Pkg/sub_package/foo.py') + + def test_isfile_case_2(self) -> None: + self.make_file('bar.py') + self.make_file('pkg/sub_package/__init__.py') + self.make_file('pkg/sub_package/foo.py') + # Run twice to test bost cached and non-cached code paths. + # This reverses the order of checks from test_isfile_case_1. + for i in range(2): + assert not self.isfile_case('Bar.py') + assert not self.isfile_case('pkg/sub_package/__init__.PY') + assert not self.isfile_case('pkg/Sub_Package/foo.py') + assert not self.isfile_case('Pkg/sub_package/foo.py') + for i in range(2): + assert self.isfile_case('bar.py') + assert self.isfile_case('pkg/sub_package/__init__.py') + assert self.isfile_case('pkg/sub_package/foo.py') + assert not self.isfile_case('non_existent.py') + assert not self.isfile_case('pkg/non_existent.py') + + def test_isfile_case_other_directory(self) -> None: + self.make_file('bar.py') + with tempfile.TemporaryDirectory() as other: + self.make_file('other_dir.py', base=other) + self.make_file('pkg/other_dir.py', base=other) + assert self.isfile_case(os.path.join(other, 'other_dir.py')) + assert not self.isfile_case(os.path.join(other, 'Other_Dir.py')) + assert not self.isfile_case(os.path.join(other, 'bar.py')) + if sys.platform in ('win32', 'darwin'): + # We only check case for directories under our prefix, and since + # this path is not under the prefix, case difference is fine. + assert self.isfile_case(os.path.join(other, 'PKG/other_dir.py')) + + def make_file(self, path: str, base: Optional[str] = None) -> None: + if base is None: + base = self.tempdir + fullpath = os.path.join(base, path) + os.makedirs(os.path.dirname(fullpath), exist_ok=True) + if not path.endswith('/'): + with open(fullpath, 'w') as f: + f.write('# test file') + + def isfile_case(self, path: str) -> bool: + return self.fscache.isfile_case(os.path.join(self.tempdir, path), self.tempdir) From 58c4f5f6560be15d016e4e7814f99becf975d7d5 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 15 Feb 2021 18:45:48 +0000 Subject: [PATCH 2/3] Address feedback --- mypy/test/testfscache.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/mypy/test/testfscache.py b/mypy/test/testfscache.py index 86c808ccaccbf..73f926ad748c5 100644 --- a/mypy/test/testfscache.py +++ b/mypy/test/testfscache.py @@ -25,7 +25,7 @@ def test_isfile_case_1(self) -> None: self.make_file('bar.py') self.make_file('pkg/sub_package/__init__.py') self.make_file('pkg/sub_package/foo.py') - # Run twice to test bost cached and non-cached code paths. + # Run twice to test both cached and non-cached code paths. for i in range(2): assert self.isfile_case('bar.py') assert self.isfile_case('pkg/sub_package/__init__.py') @@ -44,7 +44,7 @@ def test_isfile_case_2(self) -> None: self.make_file('bar.py') self.make_file('pkg/sub_package/__init__.py') self.make_file('pkg/sub_package/foo.py') - # Run twice to test bost cached and non-cached code paths. + # Run twice to test both cached and non-cached code paths. # This reverses the order of checks from test_isfile_case_1. for i in range(2): assert not self.isfile_case('Bar.py') @@ -58,6 +58,22 @@ def test_isfile_case_2(self) -> None: assert not self.isfile_case('non_existent.py') assert not self.isfile_case('pkg/non_existent.py') + def test_isfile_case_3(self) -> None: + self.make_file('bar.py') + self.make_file('pkg/sub_package/__init__.py') + self.make_file('pkg/sub_package/foo.py') + # Run twice to test both cached and non-cached code paths. + for i in range(2): + assert self.isfile_case('bar.py') + assert not self.isfile_case('non_existent.py') + assert not self.isfile_case('pkg/non_existent.py') + assert not self.isfile_case('Bar.py') + assert not self.isfile_case('pkg/sub_package/__init__.PY') + assert not self.isfile_case('pkg/Sub_Package/foo.py') + assert not self.isfile_case('Pkg/sub_package/foo.py') + assert self.isfile_case('pkg/sub_package/__init__.py') + assert self.isfile_case('pkg/sub_package/foo.py') + def test_isfile_case_other_directory(self) -> None: self.make_file('bar.py') with tempfile.TemporaryDirectory() as other: From ace790ad867bccfb52c458a9244ea96a3a68f2cc Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 15 Feb 2021 18:49:29 +0000 Subject: [PATCH 3/3] Attempt to fix Windows --- mypy/fscache.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mypy/fscache.py b/mypy/fscache.py index ef6149338a0e1..aa688868cc71b 100644 --- a/mypy/fscache.py +++ b/mypy/fscache.py @@ -229,11 +229,10 @@ def _exists_case(self, path: str, prefix: str) -> bool: if path in self.exists_case_cache: return self.exists_case_cache[path] head, tail = os.path.split(path) - if not head.startswith(prefix): + if not head.startswith(prefix) or not tail: # Only perform the check for paths under prefix. self.exists_case_cache[path] = True return True - assert tail try: names = self.listdir(head) # This allows one to check file name case sensitively in