perf: avoid redundant hash verification for cached wheel files by abhiyadav2345 · Pull Request #13860 · pypa/pip

abhiyadav2345 · 2026-03-22T15:01:05Z

Summary

Fixes #12589 — RequirementPreparer was hashing large wheel files twice when they were already present in the download directory cache.

Root Cause

When prepare_linked_requirement finds a wheel in download_dir, it calls _check_download_dir, which calls hashes.check_against_path() — reading and hashing the entire file (pass 1). The path is added to self._downloaded.

Then _prepare_linked_requirement is called. In its else branch (the "already downloaded" path), it calls hashes.check_against_path() again on the same file — a full second read (pass 2).

For multi-GB wheels (common in ML/data science), this means pip reads the entire file from disk twice just for hash verification.

The same double-hash occurs in the prepare_linked_requirements_more → _prepare_linked_requirement flow.

Fix

Add self._hash_verified: set[str] to RequirementPreparer to track URLs whose files have already passed hash verification in _check_download_dir.

In prepare_linked_requirement and prepare_linked_requirements_more: when _check_download_dir returns a path and hashes is non-empty, record the URL as verified.
In _prepare_linked_requirement's else branch: skip hashes.check_against_path() when the URL is already in self._hash_verified.

Changes

src/pip/_internal/operations/prepare.py — add _hash_verified set; populate it after _check_download_dir passes; skip redundant check in _prepare_linked_requirement
tests/unit/test_operations_prepare.py — add TestCheckDownloadDir with 5 tests covering the normal paths and the regression case
news/12589.bugfix.rst — changelog entry

Testing

uv run --group test pytest tests/unit/test_operations_prepare.py -v
# 9 passed

When a wheel is found in the download directory by _check_download_dir and its hash is verified there, _prepare_linked_requirement was unconditionally calling hashes.check_against_path() a second time on the same file in its else-branch. For large wheels (e.g. GB-sized ML packages) this means reading the entire file from disk twice. Fix by adding self._hash_verified: set[str] to RequirementPreparer to track URLs whose files have already passed hash verification. When _check_download_dir returns a path and hashes were provided, the URL is recorded as verified. _prepare_linked_requirement then skips the redundant check_against_path call for those URLs. The same tracking is applied in prepare_linked_requirements_more, which shares the same _check_download_dir -> _prepare_linked_requirement flow. Fixes pypa#12589

notatallshaw · 2026-03-22T15:08:21Z

Thanks for the PR, please be aware review capacity is very low for pip right now, but at a first glance this PR looks quite promising.

Please let us know if you need any help passing linting: https://pip.pypa.io/en/latest/development/getting-started/#running-linters

uranusjr · 2026-03-23T06:29:30Z

                if file_path is not None:
                    self._downloaded[req.link.url] = file_path
+                    if hashes:
+                        self._hash_verified.add(req.link.url)


Do these lines always appear together? I wonder if in self._hash_verified would simply be equivalent to in self._downloaded.

Good question — they're not equivalent, and the distinction matters for correctness.

_downloaded is populated in three places:

Line 491 — after a fresh batch download (_complete_partial_requirements). The file was just fetched from the network; _check_download_dir was never called, so no hash has been verified yet.

Lines 532/565 — when _check_download_dir finds the wheel already in the download cache. Only here is the hash verified upfront.

In case 2, _hash_verified is also populated — but only when hashes is non-empty.

At line 643, the else branch is reached for all URLs in _downloaded — including those from case 1 (fresh download). If we replaced link.url not in self._hash_verified with link.url not in self._downloaded, we'd skip check_against_path for freshly-downloaded files too, silently bypassing hash verification — a security regression.

_hash_verified is intentionally narrower: it only tracks URLs where _check_download_dir already did the work, so the skip at line 643 is safe to apply.

sepehr-rs

Hi @abhiyadav2345, thanks for the contribution!
Overall the PR looks good, but there are a few points worth addressing and a couple of design details to reconsider before it’s ready to be merged.

sepehr-rs · 2026-06-04T15:26:33Z

+
+        # Simulate _prepare_linked_requirement's else-branch:
+        # it should skip check_against_path when the URL is already verified.
+        if hashes and link.url not in verified_urls:


Why are we duplicating _prepare_linked_requirement's else branch here? This test wouldn’t fail if the _hash_verified optimization were removed. It might be stronger to assert call count through the actual RequirementPreparer flow instead.

sepehr-rs · 2026-06-04T15:33:18Z

+    def test_returns_none_when_file_absent(self, tmpdir: Path) -> None:
+        link = Link("https://example.com/pkg-1.0.whl")
+        result = _check_download_dir(link, os.fspath(tmpdir), hashes=None)
+        assert result is None
+
+    def test_returns_path_when_file_present_no_hashes(self, tmpdir: Path) -> None:
+        content = b"wheel content"
+        self._make_file(tmpdir, "pkg-1.0.whl", content)
+        link = Link("https://example.com/pkg-1.0.whl")
+        result = _check_download_dir(link, os.fspath(tmpdir), hashes=None)
+        assert result == os.fspath(tmpdir / "pkg-1.0.whl")
+
+    def test_returns_path_and_verifies_hash_when_file_present(
+        self, tmpdir: Path
+    ) -> None:
+        content = b"wheel content"
+        self._make_file(tmpdir, "pkg-1.0.whl", content)
+        link = Link("https://example.com/pkg-1.0.whl")
+        hashes = self._sha256_hashes(content)
+
+        with patch.object(
+            hashes, "check_against_path", wraps=hashes.check_against_path
+        ) as mock_check:
+            result = _check_download_dir(link, os.fspath(tmpdir), hashes=hashes)
+
+        assert result == os.fspath(tmpdir / "pkg-1.0.whl")
+        mock_check.assert_called_once()
+
+    def test_deletes_file_and_returns_none_on_hash_mismatch(self, tmpdir: Path) -> None:
+        content = b"wheel content"
+        self._make_file(tmpdir, "pkg-1.0.whl", content)
+        link = Link("https://example.com/pkg-1.0.whl")
+        bad_hashes = Hashes({"sha256": ["deadbeef" * 8]})
+
+        result = _check_download_dir(link, os.fspath(tmpdir), hashes=bad_hashes)
+
+        assert result is None
+        assert not (tmpdir / "pkg-1.0.whl").exists()


Why are we testing _check_download_dir here? It wasn't changed in this PR. I’d lean toward keeping this PR focused on the regression in RequirementPreparer, unless there’s intent to expand coverage here as part of this change.

sepehr-rs · 2026-06-04T15:50:13Z

+        # URLs whose cached files have already passed hash verification in
+        # _check_download_dir, so _prepare_linked_requirement can skip the
+        # redundant second check_against_path call for those files.
+        self._hash_verified: set[str] = set()


Small nitpick here: This reads a bit like a boolean flag, even though it tracks a set of URLs; maybe _verified_hash_urls (or similar) would reflect that more clearly.

psf-chronographer Bot added the bot:chronographer:provided label Mar 22, 2026

abhiyadav2345 mentioned this pull request Mar 22, 2026

Performance Issue: Too many hashes in RequirementPreparer #12589

Open

1 task

style: fix black formatting and remove unused import in test file

5e05f50

uranusjr reviewed Mar 23, 2026

View reviewed changes

sepehr-rs suggested changes Jun 4, 2026

View reviewed changes

This was referenced Jun 29, 2026

perf(cache): use a set of tags for wheel cache lookup #14122

Merged

perf(index): filter find-links entries before url conversion #14123

Open

Navigation Menu

Uh oh!

perf: avoid redundant hash verification for cached wheel files - #13860

perf: avoid redundant hash verification for cached wheel files#13860
abhiyadav2345 wants to merge 2 commits into
pypa:mainfrom
abhiyadav2345:fix/avoid-redundant-hash-check

abhiyadav2345 commented Mar 22, 2026

Uh oh!

notatallshaw commented Mar 22, 2026

Uh oh!

uranusjr Mar 23, 2026

Uh oh!

abhiyadav2345 Mar 23, 2026

Uh oh!

sepehr-rs left a comment

Uh oh!

sepehr-rs Jun 4, 2026

Uh oh!

sepehr-rs Jun 4, 2026

Uh oh!

sepehr-rs Jun 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

abhiyadav2345 commented Mar 22, 2026

Summary

Root Cause

Fix

Changes

Testing

Uh oh!

notatallshaw commented Mar 22, 2026

Uh oh!

uranusjr Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

abhiyadav2345 Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

sepehr-rs left a comment

Choose a reason for hiding this comment

Uh oh!

sepehr-rs Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

sepehr-rs Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

sepehr-rs Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants