Skip to content

config: avoid stat storm in _getconftestmodules #9532

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 25, 2022
Merged

config: avoid stat storm in _getconftestmodules #9532

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/_pytest/config/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,14 @@ def _prepareconfig(
raise


def _get_directory(path: Path) -> Path:
"""Get the directory of a path - itself if already a directory."""
if path.is_file():
return path.parent
else:
return path


@final
class PytestPluginManager(PluginManager):
"""A :py:class:`pluggy.PluginManager <pluggy.PluginManager>` with
Expand Down Expand Up @@ -357,6 +365,11 @@ def __init__(self) -> None:
# If set, conftest loading is skipped.
self._noconftest = False

# _getconftestmodules()'s call to _get_directory() causes a stat
# storm when it's called potentially thousands of times in a test
# session (#9478), often with the same path, so cache it.
self._get_directory = lru_cache(256)(_get_directory)
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to remove the bounds here, using None instead of 256?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just remembered I forgot to answer this.

I think it's always good to have limits on lru caches to avoid them growing too large. We can increase it though if it's too low for someone.


self._duplicatepaths: Set[Path] = set()

# plugins that were explicitly skipped with pytest.skip
Expand Down Expand Up @@ -547,10 +560,7 @@ def _getconftestmodules(
if self._noconftest:
return []

if path.is_file():
directory = path.parent
else:
directory = path
directory = self._get_directory(path)

# Optimization: avoid repeated searches in the same directory.
# Assumes always called with same importmode and rootpath.
Expand Down