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

python: fix quadratic behavior in collection of items using xunit fixtures #7929

Merged
merged 1 commit into from
Oct 23, 2020

Conversation

bluetech
Copy link
Member

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):

         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)

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
@bluetech
Copy link
Member Author

Thanks for the reviews! I added comments "Use a unique name to speed up lookup.".

@bluetech bluetech merged commit 0b14350 into pytest-dev:master Oct 23, 2020
@bluetech
Copy link
Member Author

I forgot to add a changelog, will add in the next PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants