From fd7ce4dbefbb4ce229a414ec28bcbebdec4c7dde Mon Sep 17 00:00:00 2001 From: mrbean-bremen Date: Sun, 13 Oct 2024 09:38:45 +0200 Subject: [PATCH] Performance: do not reload tempfile - patch tempfile instead directly (Posix only) --- CHANGES.md | 11 ++++- pyfakefs/fake_filesystem_unittest.py | 43 +++++++++++++++++-- .../tests/fake_filesystem_unittest_test.py | 41 ++++++++---------- 3 files changed, 67 insertions(+), 28 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..20c3955d 100644 --- a/pyfakefs/fake_filesystem_unittest.py +++ b/pyfakefs/fake_filesystem_unittest.py @@ -95,6 +95,41 @@ 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 + + def patchfs( _func: Optional[Callable] = None, *, @@ -576,6 +611,7 @@ def __init__( self.patch_open_code = patch_open_code self.linecache_updatecache = None self.linecache_checkcache = None + self.tempfile_patcher = TempfilePatcher() if additional_skip_names is not None: skip_names = [ @@ -590,9 +626,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 @@ -977,6 +1011,8 @@ def start_patching(self) -> None: self.linecache_checkcache = linecache.checkcache linecache.checkcache = self.checkcache + self.tempfile_patcher.start_patching() + self.patch_modules() self.patch_functions() self.patch_defaults() @@ -1075,6 +1111,7 @@ 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) + self.tempfile_patcher.stop_patching() if self.linecache_updatecache is not None: linecache.updatecache = self.linecache_updatecache linecache.checkcache = self.linecache_checkcache 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):