From d6aabc7739be473982401658d4b9590641d6fcd4 Mon Sep 17 00:00:00 2001 From: mrbean-bremen Date: Sat, 12 Oct 2024 20:04:44 +0200 Subject: [PATCH] Performance: do not reload tempfile - patch tempfile instead directly (Posix only) --- CHANGES.md | 11 +- pyfakefs/fake_filesystem_unittest.py | 105 +++++++++++++----- .../tests/fake_filesystem_unittest_test.py | 41 +++---- 3 files changed, 105 insertions(+), 52 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 2a940274..b75d3f63 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -14,6 +14,9 @@ The released versions correspond to PyPI releases. ## Unreleased +### Performance +* avoid reloading `tempfile` in Posix systems + ### Fixes * fixes a regression that caused unfaked `fcntl` calls to fail (see [#1074](../../issues/1074)) @@ -27,7 +30,9 @@ Adds official Python 3.13 support, improves OS emulation behavior. * the `additional_skip_names` parameter now works with more modules (see [#1023](../../issues/1023)) * added support for `os.fchmod`, allow file descriptor argument for `os.chmod` only for POSIX for Python < 3.13 -* avoid reloading `glob` in Python 3.13 (did affect test performance) + +### Performance +* avoid reloading `glob` in Python 3.13 ### Fixes * removing files while iterating over `scandir` results is now possible (see [#1051](../../issues/1051)) @@ -541,7 +546,7 @@ This is a bugfix release that fixes a regression introduced in version 4.2.0. This is an update to the performance release, with more setup caching and the possibility to disable it. -### Changes +### Performance * Added caching of patched modules to avoid lookup overhead * Added `use_cache` option and `clear_cache` method to be able to deal with unwanted side effects of the newly introduced caching @@ -568,6 +573,8 @@ release. default to avoid a large performance impact. An additional parameter `patch_default_args` has been added that switches this behavior on (see [#567](../../issues/567)). + +### Performance * Added performance improvements in the test setup, including caching the unpatched modules diff --git a/pyfakefs/fake_filesystem_unittest.py b/pyfakefs/fake_filesystem_unittest.py index 6465b420..c2c776f0 100644 --- a/pyfakefs/fake_filesystem_unittest.py +++ b/pyfakefs/fake_filesystem_unittest.py @@ -95,6 +95,77 @@ PATH_MODULE = "ntpath" if sys.platform == "win32" else "posixpath" +class TempfilePatcher: + """Handles tempfile patching for Posix systems.""" + + def __init__(self): + self.tempfile_cleanup = None + + def start_patching(self): + if self.tempfile_cleanup is not None: + return + if sys.version_info >= (3, 12): + + def cleanup(self_, windows=(os.name == "nt"), unlink=None): + self.tempfile_cleanup(self_, windows, unlink or os.unlink) + + self.tempfile_cleanup = tempfile._TemporaryFileCloser.cleanup # type: ignore[module-attr] + tempfile._TemporaryFileCloser.cleanup = cleanup # type: ignore[module-attr] + elif sys.platform != "win32": + + def close(self_, unlink=None): + self.tempfile_cleanup(self_, unlink or os.unlink) + + self.tempfile_cleanup = tempfile._TemporaryFileCloser.close # type: ignore[module-attr] + tempfile._TemporaryFileCloser.close = close # type: ignore[module-attr] + + def stop_patching(self): + if self.tempfile_cleanup is None: + return + if sys.version_info < (3, 12): + tempfile._TemporaryFileCloser.close = self.tempfile_cleanup # type: ignore[module-attr] + else: + tempfile._TemporaryFileCloser.cleanup = self.tempfile_cleanup # type: ignore[module-attr] + # reset the cached tempdir in tempfile + tempfile.tempdir = None + + +class LinecachePatcher: + """Handles linecache patching for Python >= 3.13.""" + + def __init__(self): + self.linecache_checkcache = None + self.linecache_updatecache = None + + def checkcache(self, filename=None): + """Calls the original linecache.checkcache making sure no fake OS calls + are used.""" + with use_original_os(): + return self.linecache_checkcache(filename) + + def updatecache(self, filename, module_globals=None): + """Calls the original linecache.updatecache making sure no fake OS calls + are used.""" + with use_original_os(): + return self.linecache_updatecache(filename, module_globals) + + def start_patching(self): + if self.linecache_updatecache is not None: + return + if sys.version_info >= (3, 13): + # in linecache, 'os' is now imported locally, which involves the + # dynamic patcher, therefore we patch the affected functions + self.linecache_updatecache = linecache.updatecache + linecache.updatecache = self.updatecache + self.linecache_checkcache = linecache.checkcache + linecache.checkcache = self.checkcache + + def stop_patching(self): + if self.linecache_updatecache is not None: + linecache.updatecache = self.linecache_updatecache + linecache.checkcache = self.linecache_checkcache + + def patchfs( _func: Optional[Callable] = None, *, @@ -574,8 +645,8 @@ def __init__( # save the original open function for use in pytest plugin self.original_open = open self.patch_open_code = patch_open_code - self.linecache_updatecache = None - self.linecache_checkcache = None + self.linecache_patcher = LinecachePatcher() + self.tempfile_patcher = TempfilePatcher() if additional_skip_names is not None: skip_names = [ @@ -590,9 +661,7 @@ def __init__( self._init_fake_module_classes() # reload tempfile under posix to patch default argument - self.modules_to_reload: List[ModuleType] = ( - [] if sys.platform == "win32" else [tempfile] - ) + self.modules_to_reload: List[ModuleType] = [] if modules_to_reload is not None: self.modules_to_reload.extend(modules_to_reload) self.patch_default_args = patch_default_args @@ -649,18 +718,6 @@ def __init__( self._patching = False self._paused = False - def checkcache(self, filename=None): - """Calls the original linecache.checkcache making sure no fake OS calls - are used.""" - with use_original_os(): - return self.linecache_checkcache(filename) - - def updatecache(self, filename, module_globals=None): - """Calls the original linecache.updatecache making sure no fake OS calls - are used.""" - with use_original_os(): - return self.linecache_updatecache(filename, module_globals) - @classmethod def clear_fs_cache(cls) -> None: """Clear the module cache.""" @@ -969,13 +1026,8 @@ def start_patching(self) -> None: self._patching = True self._paused = False - if sys.version_info >= (3, 13): - # in linecache, 'os' is now imported locally, which involves the - # dynamic patcher, therefore we patch the affected functions - self.linecache_updatecache = linecache.updatecache - linecache.updatecache = self.updatecache - self.linecache_checkcache = linecache.checkcache - linecache.checkcache = self.checkcache + self.linecache_patcher.start_patching() + self.tempfile_patcher.start_patching() self.patch_modules() self.patch_functions() @@ -1075,9 +1127,8 @@ def stop_patching(self, temporary=False) -> None: if self.use_dynamic_patch and self._dyn_patcher: self._dyn_patcher.cleanup() sys.meta_path.pop(0) - if self.linecache_updatecache is not None: - linecache.updatecache = self.linecache_updatecache - linecache.checkcache = self.linecache_checkcache + self.tempfile_patcher.stop_patching() + self.linecache_patcher.stop_patching() self._set_glob_os_functions() @property diff --git a/pyfakefs/tests/fake_filesystem_unittest_test.py b/pyfakefs/tests/fake_filesystem_unittest_test.py index 4b3de171..a812c5c0 100644 --- a/pyfakefs/tests/fake_filesystem_unittest_test.py +++ b/pyfakefs/tests/fake_filesystem_unittest_test.py @@ -541,13 +541,8 @@ def test_non_root_behavior(self): class PauseResumeTest(fake_filesystem_unittest.TestCase): def setUp(self): - self.real_temp_file = None self.setUpPyfakefs() - def tearDown(self): - if self.real_temp_file is not None: - self.real_temp_file.close() - def test_pause_resume(self): fake_temp_file = tempfile.NamedTemporaryFile() self.assertTrue(self.fs.exists(fake_temp_file.name)) @@ -555,11 +550,11 @@ def test_pause_resume(self): self.pause() self.assertTrue(self.fs.exists(fake_temp_file.name)) self.assertFalse(os.path.exists(fake_temp_file.name)) - self.real_temp_file = tempfile.NamedTemporaryFile() - self.assertFalse(self.fs.exists(self.real_temp_file.name)) - self.assertTrue(os.path.exists(self.real_temp_file.name)) + real_temp_file = tempfile.NamedTemporaryFile() + self.assertFalse(self.fs.exists(real_temp_file.name)) + self.assertTrue(os.path.exists(real_temp_file.name)) self.resume() - self.assertFalse(os.path.exists(self.real_temp_file.name)) + self.assertFalse(os.path.exists(real_temp_file.name)) self.assertTrue(os.path.exists(fake_temp_file.name)) def test_pause_resume_fs(self): @@ -572,15 +567,15 @@ def test_pause_resume_fs(self): self.fs.pause() self.assertTrue(self.fs.exists(fake_temp_file.name)) self.assertFalse(os.path.exists(fake_temp_file.name)) - self.real_temp_file = tempfile.NamedTemporaryFile() - self.assertFalse(self.fs.exists(self.real_temp_file.name)) - self.assertTrue(os.path.exists(self.real_temp_file.name)) + real_temp_file = tempfile.NamedTemporaryFile() + self.assertFalse(self.fs.exists(real_temp_file.name)) + self.assertTrue(os.path.exists(real_temp_file.name)) # pause does nothing if already paused self.fs.pause() - self.assertFalse(self.fs.exists(self.real_temp_file.name)) - self.assertTrue(os.path.exists(self.real_temp_file.name)) + self.assertFalse(self.fs.exists(real_temp_file.name)) + self.assertTrue(os.path.exists(real_temp_file.name)) self.fs.resume() - self.assertFalse(os.path.exists(self.real_temp_file.name)) + self.assertFalse(os.path.exists(real_temp_file.name)) self.assertTrue(os.path.exists(fake_temp_file.name)) def test_pause_resume_contextmanager(self): @@ -590,10 +585,10 @@ def test_pause_resume_contextmanager(self): with Pause(self): self.assertTrue(self.fs.exists(fake_temp_file.name)) self.assertFalse(os.path.exists(fake_temp_file.name)) - self.real_temp_file = tempfile.NamedTemporaryFile() - self.assertFalse(self.fs.exists(self.real_temp_file.name)) - self.assertTrue(os.path.exists(self.real_temp_file.name)) - self.assertFalse(os.path.exists(self.real_temp_file.name)) + real_temp_file = tempfile.NamedTemporaryFile() + self.assertFalse(self.fs.exists(real_temp_file.name)) + self.assertTrue(os.path.exists(real_temp_file.name)) + self.assertFalse(os.path.exists(real_temp_file.name)) self.assertTrue(os.path.exists(fake_temp_file.name)) def test_pause_resume_fs_contextmanager(self): @@ -603,10 +598,10 @@ def test_pause_resume_fs_contextmanager(self): with Pause(self.fs): self.assertTrue(self.fs.exists(fake_temp_file.name)) self.assertFalse(os.path.exists(fake_temp_file.name)) - self.real_temp_file = tempfile.NamedTemporaryFile() - self.assertFalse(self.fs.exists(self.real_temp_file.name)) - self.assertTrue(os.path.exists(self.real_temp_file.name)) - self.assertFalse(os.path.exists(self.real_temp_file.name)) + real_temp_file = tempfile.NamedTemporaryFile() + self.assertFalse(self.fs.exists(real_temp_file.name)) + self.assertTrue(os.path.exists(real_temp_file.name)) + self.assertFalse(os.path.exists(real_temp_file.name)) self.assertTrue(os.path.exists(fake_temp_file.name)) def test_pause_resume_without_patcher(self):