fixtures: use a better data structure for storing FixtureDefs

[bluetech]

This PR fixes #14942.

The commits

The 1st and 2nd commits are related internal documentation fixes.

The 3rd commit is a little refactor to encapsulate access to FixtureManager._arg2fixturedef to make the next commit clearer.

The 4th commit is main change, I reproduce the commit message below.

The 5th commit adds backward compat for string nodeids in register_fixture. I split it from the previous commit to make it easier to review the main change without the ugly compat. I will squash it before merging.

Description

Previously, FixtureManager stored the registered FixtureDefs in _arg2fixturedefs which is

<fixutre name> -> [FixtureDef]

where the FixtureDefs are ordered by visibility.

There are two inefficiencies with this:

1. When registering a fixture, we need to find the appropriate index to insert in the list. This is done with slow quadratic is_visibility_more_specific checks.

   Before 7186cd4, FixtureDefs were always appended, relying on the collection order, so there was no quadratic issue. But then we added pytest.register_fixture which is not guaranteed to be called in collection order.

   This is Test collection is 10x slower on 9.1.1 compared to 9.0.3 #14942, introduced in v9.1.0.

2. When looking up FixtureDefs for a node, the entire list needed to be filtered for visibility to the node (_matchfactories). With many fixtures registered with the same name (even if completely unrelated), this can be slow.

   This is an old issue.

Change the way we store the FixtureDefs to _arg2node2fixturedefs, which is

<fixture name> -> (Node -> [FixtureDef])

i.e. instead of storing the FixtureDefs for a name in a single big list, store them by the Node under which they are registered.

This fixes (1) since now just need to append to arg2fixture2nodes[name][node].

Fixes (2) since no longer need to filter a big list (scaling with number of fixtures registered with same name). Instead need to look up the FixtureDefs registered for the node and its ancestors (scales with height of the collection tree, which should be OK).

Performance

Performance numbers with reproducer from #14942 (comment) (NCLASSES=10000 time pytest reproducer.py):

• This PR: 11.0s
• main: 35.0s
• Commit before 7186cd4: 16.7s

It's still kinda slow, but a profile shows that's for other preexisting reasons. The improvement over main is due to fixing problem (1), and over before the regression due to fixing problem (2).

Backward compat

This breaks plugins which directly access FixtureManager._arg2fixturedefs (double private 😀). From my local corpus (678 plugins), I see it done in these plugins:

• pytest-bdd - Moderately elaborate usage. I will try to send a PR to fix if this is merged.
• pytest-deadfixtures - Just iterates over all FixtureDefs. I will send them a PR if this merged.
• python-pytest-steps - getting some downloads but hasn't been updated in last 5 years. I will ask smarie about it if this is merged.
• pytest-airflow - not popular
• pytest-eucalyptus - not popular
• pytest-exploratory - not popular
• pytest-missing-fixtures - not popular
• pytest-tipsi-django - not popular

I think this is acceptable.

[nicoddemus]

Great work!

[nicoddemus]

Suggested change

	Then can particularly happen when many tests are defined in separate classes which inherit from a base class which defines fixtures (the fixtures are repeated per class).
	This can happen particularly when many tests are defined in separate classes which inherit from a base class which defines fixtures (the fixtures are repeated per class).

[nicoddemus]

We could use itertools.chain instead of building a list, might be a bit more efficient (or maybe not that much, given the list of fixturedefs is much smaller now).

[nicoddemus]

Instead of a separate datastructure, is it possible for us to get the nodeid from the session collection tree? Or perhaps the node might not exist at this point yet?