Skip to content

Commit

Permalink
Performance: do not reload tempfile
Browse files Browse the repository at this point in the history
- patch tempfile instead directly (Posix only)
  • Loading branch information
mrbean-bremen committed Oct 13, 2024
1 parent 305ed2e commit fd7ce4d
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 28 deletions.
11 changes: 9 additions & 2 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand All @@ -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))
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down
43 changes: 40 additions & 3 deletions pyfakefs/fake_filesystem_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
*,
Expand Down Expand Up @@ -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 = [
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
41 changes: 18 additions & 23 deletions pyfakefs/tests/fake_filesystem_unittest_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -541,25 +541,20 @@ 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))
self.assertTrue(os.path.exists(fake_temp_file.name))
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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand Down

0 comments on commit fd7ce4d

Please sign in to comment.