perf: cache __hash__ on Specifier, Marker, and Requirement - #1252
Draft
henryiii wants to merge 1 commit into
Draft
perf: cache __hash__ on Specifier, Marker, and Requirement#1252henryiii wants to merge 1 commit into
henryiii wants to merge 1 commit into
Conversation
Specifier.__hash__ recomputed _canonical_spec (parsing and canonicalizing the version) on every call, Marker.__hash__ re-serialized its whole marker tree, and Requirement.__hash__ re-serialized all of its parts. All three types are immutable, so cache the computed hash lazily in a _hash_cache attribute, mirroring the pattern Version already uses. For the slotted classes (Specifier, Marker) a _hash_cache slot is added and initialized to None on every construction path, including alternate constructors (Marker._from_markers, the Marker built directly in Requirement.__init__) and the __setstate__ pickle paths. The existing explicit __getstate__ implementations return only the core fields, so the cache is never serialized into new pickles. Requirement has no __slots__, so a plain attribute is used and initialized in __init__ and in both __setstate__ branches. Assisted-by: ClaudeCode:claude-opus-4-8
henryiii
marked this pull request as ready for review
June 12, 2026 19:07
Member
|
Requirement doesn't indicate that it should be immutable, it doesn't have I would be up for deprecating it's mutability and/or making all it's attributes properties that clear cache on writing to, but I don't think we can assume people aren't mutating it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In draft, I think we should work on making these immutable first.
🤖 AI text below 🤖
Specifier.__hash__recomputed_canonical_spec(a parse + canonicalization) on every call, andMarker.__hash__/Requirement.__hash__re-serialized their whole tree each time. All three are immutable, so this caches the hash lazily using the same_hash_cachepatternVersionadopted in #1118: the slot/attribute is initialized toNoneon every construction path (including__setstate__,Marker._from_markers, and theMarker.__new__bypass insideRequirement.__init__), and__getstate__already returns explicit state so the cache never leaks into pickles. Hash values and equality semantics are unchanged.timeit(Python 3.14, Apple M5 Pro; the asv suite has no hash benchmarks — the full suite shows no regressions):hash(Specifier)hash(Marker)hash(Requirement)SpecifierSetis deliberately untouched here; its canonicalization interacts with other caches and is handled in #1253.Part of #1239.
🤖 Generated with Claude Code