Ask the zip archive whether a member name repeats, instead of walking every member - #237
Merged
Merged
Conversation
… every member The seeker grouped all of an archive's members into a dict of name to entries purely to find the names stored more than once. ZipFile has already keyed its own NameToInfo on the member name by the time the seeker runs, so that dict is short by exactly the number of repeats. Comparing two lengths answers the same question and allocates nothing. Measured on a 630,560 member extraction: the walk cost a dict entry, a list and a tuple per member, 138 MB, to discover that no name repeated. Peak memory for a light run over that archive goes from 987 MB to 871 MB. Row counts are unchanged. Archives that do repeat a name still take the walk and are unaffected. Four tests cover the new branch, including one that makes the member walk raise so the cheap path cannot be dropped without a failure, and its control on a repeated name where the walk must run. The seeker and its test are byte-identical across the five cores, and stay so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
The zip seeker grouped every member of an archive into a dict of name to entries, purely to find the names stored more than once.
ZipFilehas already keyed its ownNameToInfoon the member name by the time the seeker runs, so that dict is short by exactly the number of repeats. Comparing two lengths answers the same question and allocates nothing.Measured on a 630,560 member extraction, where the walk cost a dict entry, a list and a tuple per member:
Three runs a side on the light profile: 1018, 985, 987 against 871, 871, 871. Row counts are unchanged, and the artifact that reads 1,208,783 rows from that archive still reads 1,208,783.
Checked on two more real archives, through the seeker itself rather than a full run:
The second takes the walk, as it must, and both trees return the same 82,592 distinct names, the same 53 chosen entries and the same 53 names carrying another version.
Archives that do repeat a name take the walk exactly as before.
NameToInfois not part of zipfile's documented surface, so an absent one falls through to the walk rather than assuming anything. It is present on 3.10 through 3.14.Four tests cover the new branch. One makes the member walk raise, so the cheap path cannot be dropped without a failure, and its control does the same on a repeated name where the walk must still run. Two mutants were used: removing the cheap path fails exactly the walk test, and taking it unconditionally fails nine of the repeated-name tests.
The seeker function and its test file are byte-identical across the five cores, and stay so after this change.
🤖 Generated with Claude Code