Skip to content

Commit

Permalink
Consistently use get_requirement to cache Requirement construction (
Browse files Browse the repository at this point in the history
  • Loading branch information
notatallshaw authored Jul 9, 2024
1 parent f0bb386 commit 9d8e765
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 20 deletions.
1 change: 1 addition & 0 deletions news/12663.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve performance when the same requirement string appears many times during resolution, by consistently caching the parsed requirement string.
4 changes: 2 additions & 2 deletions src/pip/_internal/build_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@
from typing import TYPE_CHECKING, Iterable, List, Optional, Set, Tuple, Type, Union

from pip._vendor.certifi import where
from pip._vendor.packaging.requirements import Requirement
from pip._vendor.packaging.version import Version

from pip import __file__ as pip_location
from pip._internal.cli.spinners import open_spinner
from pip._internal.locations import get_platlib, get_purelib, get_scheme
from pip._internal.metadata import get_default_environment, get_environment
from pip._internal.utils.logging import VERBOSE
from pip._internal.utils.packaging import get_requirement
from pip._internal.utils.subprocess import call_subprocess
from pip._internal.utils.temp_dir import TempDirectory, tempdir_kinds

Expand Down Expand Up @@ -184,7 +184,7 @@ def check_requirements(
else get_default_environment()
)
for req_str in reqs:
req = Requirement(req_str)
req = get_requirement(req_str)
# We're explicitly evaluating with an empty extra value, since build
# environments are not provided any mechanism to select specific extras.
if req.marker is not None and not req.marker.evaluate({"extra": ""}):
Expand Down
3 changes: 2 additions & 1 deletion src/pip/_internal/metadata/importlib/_dists.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
Wheel,
)
from pip._internal.utils.misc import normalize_path
from pip._internal.utils.packaging import get_requirement
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.utils.wheel import parse_wheel, read_wheel_metadata_file

Expand Down Expand Up @@ -219,7 +220,7 @@ def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requiremen
for req_string in self.metadata.get_all("Requires-Dist", []):
# strip() because email.message.Message.get_all() may return a leading \n
# in case a long header was wrapped.
req = Requirement(req_string.strip())
req = get_requirement(req_string.strip())
if not req.marker:
yield req
elif not extras and req.marker.evaluate({"extra": ""}):
Expand Down
5 changes: 3 additions & 2 deletions src/pip/_internal/pyproject.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
else:
from pip._vendor import tomli as tomllib

from pip._vendor.packaging.requirements import InvalidRequirement, Requirement
from pip._vendor.packaging.requirements import InvalidRequirement

from pip._internal.exceptions import (
InstallationError,
InvalidPyProjectBuildRequires,
MissingPyProjectBuildRequires,
)
from pip._internal.utils.packaging import get_requirement


def _is_list_of_str(obj: Any) -> bool:
Expand Down Expand Up @@ -156,7 +157,7 @@ def load_pyproject_toml(
# Each requirement must be valid as per PEP 508
for requirement in requires:
try:
Requirement(requirement)
get_requirement(requirement)
except InvalidRequirement as error:
raise InvalidPyProjectBuildRequires(
package=req_name,
Expand Down
6 changes: 3 additions & 3 deletions src/pip/_internal/req/constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def _set_requirement_extras(req: Requirement, new_extras: Set[str]) -> Requireme
pre is not None and post is not None
), f"regex group selection for requirement {req} failed, this should never happen"
extras: str = "[%s]" % ",".join(sorted(new_extras)) if new_extras else ""
return Requirement(f"{pre}{extras}{post}")
return get_requirement(f"{pre}{extras}{post}")


def parse_editable(editable_req: str) -> Tuple[Optional[str], str, Set[str]]:
Expand Down Expand Up @@ -163,7 +163,7 @@ def check_first_requirement_in_file(filename: str) -> None:
# If there is a line continuation, drop it, and append the next line.
if line.endswith("\\"):
line = line[:-2].strip() + next(lines, "")
Requirement(line)
get_requirement(line)
return


Expand Down Expand Up @@ -205,7 +205,7 @@ def parse_req_from_editable(editable_req: str) -> RequirementParts:

if name is not None:
try:
req: Optional[Requirement] = Requirement(name)
req: Optional[Requirement] = get_requirement(name)
except InvalidRequirement as exc:
raise InstallationError(f"Invalid requirement: {name!r}: {exc}")
else:
Expand Down
5 changes: 3 additions & 2 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
redact_auth_from_requirement,
redact_auth_from_url,
)
from pip._internal.utils.packaging import get_requirement
from pip._internal.utils.subprocess import runner_with_spinner_message
from pip._internal.utils.temp_dir import TempDirectory, tempdir_kinds
from pip._internal.utils.unpacking import unpack_file
Expand Down Expand Up @@ -395,7 +396,7 @@ def _set_requirement(self) -> None:
else:
op = "==="

self.req = Requirement(
self.req = get_requirement(
"".join(
[
self.metadata["Name"],
Expand All @@ -421,7 +422,7 @@ def warn_on_mismatching_name(self) -> None:
metadata_name,
self.name,
)
self.req = Requirement(metadata_name)
self.req = get_requirement(metadata_name)

def check_if_exists(self, use_user_site: bool) -> None:
"""Find an installed distribution that satisfies or conflicts
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/utils/packaging.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def check_requires_python(
return python_version in requires_python_specifier


@functools.lru_cache(maxsize=512)
@functools.lru_cache(maxsize=2048)
def get_requirement(req_string: str) -> Requirement:
"""Construct a packaging.Requirement object with caching"""
# Parsing requirement strings is expensive, and is also expected to happen
Expand Down
28 changes: 19 additions & 9 deletions tests/unit/test_req.py
Original file line number Diff line number Diff line change
Expand Up @@ -770,11 +770,16 @@ def test_install_req_drop_extras(self, inp: str, out: str) -> None:
without_extras = install_req_drop_extras(req)
assert not without_extras.extras
assert str(without_extras.req) == out
# should always be a copy
assert req is not without_extras
assert req.req is not without_extras.req

# if there are no extras they should be the same object,
# otherwise they may be a copy due to cache
if req.extras:
assert req is not without_extras
assert req.req is not without_extras.req

# comes_from should point to original
assert without_extras.comes_from is req

# all else should be the same
assert without_extras.link == req.link
assert without_extras.markers == req.markers
Expand All @@ -790,9 +795,9 @@ def test_install_req_drop_extras(self, inp: str, out: str) -> None:
@pytest.mark.parametrize(
"inp, extras, out",
[
("pkg", {}, "pkg"),
("pkg==1.0", {}, "pkg==1.0"),
("pkg[ext]", {}, "pkg[ext]"),
("pkg", set(), "pkg"),
("pkg==1.0", set(), "pkg==1.0"),
("pkg[ext]", set(), "pkg[ext]"),
("pkg", {"ext"}, "pkg[ext]"),
("pkg==1.0", {"ext"}, "pkg[ext]==1.0"),
("pkg==1.0", {"ext1", "ext2"}, "pkg[ext1,ext2]==1.0"),
Expand All @@ -816,9 +821,14 @@ def test_install_req_extend_extras(
assert str(extended.req) == out
assert extended.req is not None
assert set(extended.extras) == set(extended.req.extras)
# should always be a copy
assert req is not extended
assert req.req is not extended.req

# if extras is not a subset of req.extras then the extended
# requirement object should not be the same, otherwise they
# might be a copy due to cache
if not extras.issubset(req.extras):
assert req is not extended
assert req.req is not extended.req

# all else should be the same
assert extended.link == req.link
assert extended.markers == req.markers
Expand Down

0 comments on commit 9d8e765

Please sign in to comment.