Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pyfakefs 4.x is very slow #567

Closed
mrbean-bremen opened this issue Nov 15, 2020 · 1 comment
Closed

Pyfakefs 4.x is very slow #567

mrbean-bremen opened this issue Nov 15, 2020 · 1 comment

Comments

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Nov 15, 2020

The performance of pyfakefs has decreased dramatically (at least by a factor of 6) with version 4. I accidentally stumbled upon a commit that has removed pyfakefs due to this problem.
We need some performance tests in place to avoid this kind of problems--this is something that I didn't pay attention to lately.
The performance decrease in version 4 is largely due to the added handling of default arguments that contain file system functions.
As it is quite expansive to find these values, it is probably better to switch this feature off by default and add an option to switch it on only if needed. We could patch this kind of parameters manually as before (I think tempfile needed this), or switch the feature on for specific modules only.

mrbean-bremen added a commit that referenced this issue Nov 17, 2020
- remove due to large performance impact
- add new argument "patch_default_args" to re-enable this
- avoid calling the expansive inspect methods
- added preliminary performance test - performance is now as in 3.7
- see #567
@mrbean-bremen
Copy link
Member Author

With the introduced caching the setup performance is now acceptable. The performance test, which does nothing but setting up an empty test 100 times (with some modules loaded), takes now between 200 and 300 ms in master (but 3 - 3.5 s without caching), e.g 2-3 ms overhead per test. This is comparable with the performance in version 3.3 (which did do less patching).
Will make a patch release with the new caching after some additional testing.
Closing this as fixed. Performance in future versions shall be monitored.

mrbean-bremen referenced this issue in Checkmk/checkmk Nov 25, 2020
pyfakefs looked like a handy tool, but has important drawbacks:

The pyfakefs fixture is very resource hungy. The Patcher searches all
modules registered to sys.modules for access to IO functions and patches
them. In our test environment the pyfakefs fixture setup phase was
consuming half of the test execution time.

To improve this, we'll have to replace all possible uses of pyfakefs.
Especially the call sites that just need access to some dirctories or
files in a temporary directoy can easily be replaced with the standard
pytest tmp_path fixture.

For the other call sites, which really want to access hard coded system
wide paths, these are the OMD tests, we had to introduce some dynamic
path prefix. This allows us to replace pyfakefs also in for these tests
without having to refactor larger parts of the OMD code.

With this change we are now down to 2(!!) minutes with our whole
collection of unit test

```
> time pytest -T unit tests/unit/
========================================= test session starts =========================================
platform linux -- Python 3.8.5, pytest-6.1.1, py-1.9.0, pluggy-0.13.1
rootdir: /home/lm/git/checkmk, configfile: pytest.ini
plugins: testmon-1.0.3, mock-3.3.1, cov-2.10.1, profiling-1.7.0, pyfakefs-4.1.0, requests-mock-1.8.0
collected 7808 items

(...)
====================== 7794 passed, 14 skipped, 35 warnings in 113.32s (0:01:53) ======================

real	1m56,275s
user	1m18,824s
sys	0m14,788s
```

Change-Id: I3d53303830ba6103035d4e293b045dd750ad978f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant