Repository navigation
Bound PyTorch ZIP storage entry lookups - #41
Merged
Merged
Conversation
22 of 25 tasks
leehack
marked this pull request as ready for review
November 3, 2026 20:01
This was referenced Nov 4, 2026
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
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.
PyTorch ZIP metadata restarts an archive scan for every tensor storage. Each entry open also converts its timestamp, which can exceed 30 seconds on a small synthetic checkpoint before tensor data is read. Cache the first normalized name, index and size as one reader scans forward, bounding lookup scans to one archive traversal while preserving duplicate-name order and tensor byte checks. Related to #24. Accepted macOS PR-artifact diagnostics and pending final-release qualification are recorded separately below.
The maintainer explicitly approved a patch-specific exception for this ZIP lookup patch: the prepared upstream report stays local, and owner #24 tracks retirement until an upstream report is authorized. The general carried-patch policy is unchanged. No upstream issue or report was posted.
At
f9a734fe225247566ce4de5445e055172590d241, only that patch-table row differs from6f54ed2d257faa0ed27d2a93cc5d89ea268e622d; every other tracked object, including the native patch and submodule pin, is byte-identical. Fresh model-free checks pass: eight ZIP lookup regressions (linear operation counts, unpatched controls, semantic/cleanup mutants and ASan/UBSan), five pickle-span tests, and 19 tooling tests with two expected platform skips. No model or provider execution accompanied this documentation update. Renewed independent blocking review accepted this exact head with zero P1/P2 findings (receipt SHA2564af17a9a204ca513712c7464815117b541d628409a2198406e0a352f75220334). All 17 fresh-head CI checks passed, including the separately rerun iOS simulator job that initially could not acquire a hosted runner; that failure is retained.Retained metadata evidence from the unchanged runtime fix: original flat 0.235136 s and nested 0.220261 s, each with 1,329 ZIP entries, 1,323 tensors, 2,656 entry opens, zero tensor extractions and verified cleanup. Full name/dtype/shape/byte-count inventories match the freshly hash/header-verified original Safetensors file; the two native inventories match each other. The prior unchanged flat reader exceeded 400,000 opens and timed out at 30 seconds. This metadata evidence proves inventory only; the separate original-mode load/generation diagnostic below provides bounded acceptance for the PR artifact.
Accepted actual diagnostic (existing #1703 SD update): macOS arm64 PR artifact
13249907568/ run41673639935, owner source6f54ed2d257faa0ed27d2a93cc5d89ea268e622d, passed all six maintained native/public CPU cases using original Safetensors, flat PyTorch ZIP and nested ZIP with external TAESD. All six raw RGB outputs are byte-identical: SHA25657cdf29472ff1c6eae8c0eb78deb6d08bc7cf629a0139f43173da655ce16a3a8. Loads took 1.20–5.72 seconds, every case met the original 45-second load and 180-second whole-case bounds, and all 16 owned processes exited zero and were reaped without cleanup errors. Independent review verified exact source/plan/runtime bindings and raw hashes.2fdb0cb0474dbcc74c9a69ef067cdd18b7994f1c3c371373a9d91599c44a1301c3a3b8c3dca629c1b0cbfb3c1cf687bde4a3a5c88ca417b102e9fdd8ada655177036e441dd0147cec68caddccc26426f50ea8327c6c879451cac5d74395ed62a05a8760b20d61f067cf50168e53742014542e52fThis accepts that six-case PR-artifact diagnostic. Qualification of a final merged/release artifact, publication and core adoption remain separate pending gates; this is not published-artifact or broader platform/backend qualification. Earlier failed runs remain retained.
The earlier Linux CI failure was the test harness forcing strict C11 while production uses GNU11. The harness now matches production and exposes bounded compiler diagnostics; production patch bytes did not change.
Source review and CI are complete. Final merged/release-artifact qualification remains a separate owner-publication gate; the upstream report remains local.