python: fix quadratic behavior in collection of items using xunit fixtures#7929
Merged
bluetech merged 1 commit intopytest-dev:masterfrom Oct 23, 2020
Merged
Conversation
nicoddemus
approved these changes
Oct 23, 2020
Member
nicoddemus
left a comment
There was a problem hiding this comment.
Great finding @bluetech, thanks a lot!
It is probably good to leave a comment in the code explaining why the name of the fixture is different with a link to the issue.
RonnyPfannschmidt
approved these changes
Oct 23, 2020
Member
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
Lovely workaround and detailing of the underlying issue
Thanks!
…tures Since commit 0f918b1 pytest uses auto-generated autouse pytest fixtures for the xunit fixtures {setup,teardown}_{module,class,method,function}. All of these fixtures were given the same name. Unfortunately, pytest fixture lookup for a name works by grabbing all of the fixtures globally declared with a name and filtering to only those which match the specific node. So each xunit-using item iterates over a list (of fixturedefs) of a size of all previous same-xunit-using items, i.e. quadratic. Fixing this properly to use a better data structure is likely to take some effort, but we can avoid the immediate problem by just using a different name for each item's autouse fixture, so it only matches itself. A benchmark is added to demonstrate the issue. It is still way too slow after the fix and possibly still quadratic, but for a different reason which is another matter. Running --collect-only, before (snipped): 202533232 function calls (201902604 primitive calls) in 86.379 seconds ncalls tottime percall cumtime percall filename:lineno(function) 1 0.000 0.000 85.688 85.688 main.py:320(pytest_collection) 1 0.000 0.000 85.688 85.688 main.py:567(perform_collect) 80557/556 0.021 0.000 85.050 0.153 {method 'extend' of 'list' objects} 85001/15001 0.166 0.000 85.045 0.006 main.py:785(genitems) 10002 0.050 0.000 84.219 0.008 runner.py:455(collect_one_node) 10002 0.049 0.000 83.763 0.008 runner.py:340(pytest_make_collect_report) 10002 0.079 0.000 83.668 0.008 runner.py:298(from_call) 10002 0.019 0.000 83.541 0.008 runner.py:341(<lambda>) 5001 0.184 0.000 81.922 0.016 python.py:412(collect) 5000 0.020 0.000 81.072 0.016 python.py:842(collect) 30003 0.118 0.000 78.478 0.003 python.py:218(pytest_pycollect_makeitem) 30000 0.190 0.000 77.957 0.003 python.py:450(_genfunctions) 40001 0.081 0.000 76.664 0.002 nodes.py:183(from_parent) 30000 0.087 0.000 76.629 0.003 python.py:1595(from_parent) 40002 0.092 0.000 76.583 0.002 nodes.py:102(_create) 30000 0.305 0.000 76.404 0.003 python.py:1533(__init__) 15000 0.132 0.000 74.765 0.005 fixtures.py:1439(getfixtureinfo) 15000 0.165 0.000 73.664 0.005 fixtures.py:1492(getfixtureclosure) 15000 0.044 0.000 57.584 0.004 fixtures.py:1653(getfixturedefs) 30000 18.840 0.001 57.540 0.002 fixtures.py:1668(_matchfactories) 37507500 31.352 0.000 38.700 0.000 nodes.py:76(ischildnode) 15000 10.464 0.001 15.806 0.001 fixtures.py:1479(_getautousenames) 112930587/112910019 7.333 0.000 7.339 0.000 {built-in method builtins.len} After: 51890333 function calls (51259706 primitive calls) in 27.306 seconds ncalls tottime percall cumtime percall filename:lineno(function) 1 0.000 0.000 26.783 26.783 main.py:320(pytest_collection) 1 0.000 0.000 26.783 26.783 main.py:567(perform_collect) 80557/556 0.020 0.000 26.108 0.047 {method 'extend' of 'list' objects} 85001/15001 0.151 0.000 26.103 0.002 main.py:785(genitems) 10002 0.047 0.000 25.324 0.003 runner.py:455(collect_one_node) 10002 0.045 0.000 24.888 0.002 runner.py:340(pytest_make_collect_report) 10002 0.069 0.000 24.805 0.002 runner.py:298(from_call) 10002 0.017 0.000 24.690 0.002 runner.py:341(<lambda>) 5001 0.168 0.000 23.150 0.005 python.py:412(collect) 5000 0.019 0.000 22.223 0.004 python.py:858(collect) 30003 0.101 0.000 19.818 0.001 python.py:218(pytest_pycollect_makeitem) 30000 0.161 0.000 19.368 0.001 python.py:450(_genfunctions) 30000 0.302 0.000 18.236 0.001 python.py:1611(from_parent) 40001 0.084 0.000 18.051 0.000 nodes.py:183(from_parent) 40002 0.116 0.000 17.967 0.000 nodes.py:102(_create) 30000 0.308 0.000 17.770 0.001 python.py:1549(__init__) 15000 0.117 0.000 16.111 0.001 fixtures.py:1439(getfixtureinfo) 15000 0.134 0.000 15.135 0.001 fixtures.py:1492(getfixtureclosure) 15000 9.320 0.001 14.738 0.001 fixtures.py:1479(_getautousenames)
5d5c761 to
50114d4
Compare
Member
Author
|
Thanks for the reviews! I added comments "Use a unique name to speed up lookup.". |
Member
Author
|
I forgot to add a changelog, will add in the next PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #4824 (as mentioned below there is still some slowness we should look at, so not closing)
Since commit 0f918b1 pytest uses auto-generated autouse pytest fixtures for the xunit fixtures
{setup,teardown}_{module,class,method,function}. All of these fixtures were given the same name.Unfortunately, pytest fixture lookup for a name works by grabbing all of the fixtures globally declared with a name and filtering to only those which match the specific node. So each xunit-using item iterates over a list (of fixturedefs) of a size of all previous same-xunit-using items, i.e. quadratic.
Fixing this properly to use a better data structure is likely to take some effort, but we can avoid the immediate problem by just using a different name for each item's autouse fixture, so it only matches itself.
A benchmark is added to demonstrate the issue. It is still way too slow after the fix and possibly still quadratic, but for a different reason which is another matter.
Running --collect-only, before (snipped):
After: