Repository navigation
Avoid redundant clearing in legacy archive extraction - #1140
Merged
Merged
Conversation
This was referenced Nov 7, 2026
Base automatically changed from
docs/deferred-zip-compatibility-findings
to
main
November 7, 2026 23:13
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.
Remove an unnecessary 64 KiB buffer clear in legacy archive extraction. Fresh memory-TAR measurements improve on both hosts; some gzip sizes remain slower.
AG;DR
The only runtime change removes value initialization from
ReadPositionedEntry's existingstd::array. Libarchive writes the returned prefix, and the existing loop appends exactly that count after its error and output-limit checks. The ordinary streaming source path is unchanged. No branch, size heuristic, wrapper, allocation policy, dependency or other buffer initialization is changed.Two new regression cases verify exact binary payloads across empty, partial, full and multiple 64 KiB blocks; exact and insufficient limits; and large/empty/short calls that cannot expose an earlier stack tail. They pass on both parent and candidate on both hosts. Five focused release targets and three focused ASan targets passed per host; three Linux MSan targets passed. These are focused development results, not a full private suite or final public-head qualification.
Full measurement uses three independently ordered sessions, nine repetitions retaining seven per case and participant, identical-parent repeats, byte-identical native fixtures and complete content comparison/destruction. Linux is pinned to one CPU; Mac is unpinned. Medians of per-case session changes are legacy memory TAR -8.73% Mac/-3.89% Linux, native TAR -1.98%/-1.89%, and gzip -2.37%/-0.19%. These are API measurements, not a general CLI/archive-search speedup. The unchanged streaming path remains a separate control. No RSS or allocation-count improvement is asserted.
Retained cost: Mac gzip at 65,535/65,536/65,537 bytes costs +2.11/+3.33/+3.08% case medians. Exact 64 KiB is slower in every session (+3.33/+2.57/+4.48%), with parent repeats -0.39/+3.00/+0.32%. All noisy and worse cells remain, including large stream-control variability. No cache or allocator cause is established. This simple operation removal is proposed for its reproducible TAR API gains with those explicit costs; no unqualified size-based workaround is added. O07's different buffer decision remains separate.
A valid empty gzip is already rejected by the unchanged legacy API before reaching this buffer. Its 81 expected-error observations per host remain explicit availability data without a timing ratio, and TODO tracks that defect separately. The initial full Mac run stopped on this precondition and resumed only missing cases; successful observations were neither repeated nor overwritten.
The report
docs/benchmarks/archive-read-buffer-clear-20261107.mdand its 1,106,635-byte lossless capsule retain 2,114 hashed records, including every sample, source/configuration/image identity, fixture, plan, fixed order, setup failure and sanitizer/test receipt. SHA-256:c86d0b03d23f71cb334ee813b6d90eb98524dfe3c6082906cf6bd3f2c1b79af7. Independent review decoded every record and recomputed every full case/session ratio from all 2,511 timing observations and 81 error controls per host; the published medians and slower sessions reproduce. Research benchmark targets and sources stay outside production.Signed public head
ac4e69b2f434126ca17fb500322c181565f92c41is a separate child of #1139 head13afa43c14fac85c4ae8333d0ad5ee5e43b56727. This is soft ancestry for tracking/site overlap, with no hard API dependency. Exactly ten files include the nine-file frozen payload integration plus a concise CHANGELOG entry; newer canonical sections, asset exceptions and site mappings are preserved. The runtime, BUILD, new test, report and capsule bytes match the qualified payload, and all ten integrated files stayed unchanged through normal signed commit/push hooks and Trunk. All 32 integrated release-site tests passed under Python 3.13. No actual local clang-tidy ran: the configured wrapper exits at its mandatory missing-compilation-database guard before executable discovery. Own fresh current-head CI and eventual main-context validation remain required. The narrow integration/main task stays unchecked until that qualification; ready parents must not wait for this child.Current-main synchronization and fresh qualification
Per the requested sequence, qualify #1140 first, then #1141, #1142 and #1144. Signed merge
1d2a6901c46719059e1c6eb584a0025bb344fab2synchronizes this PR with actual main3e546dbef0993486217eb58f3f9276dd0003651c(including merged #1143 and #1139). Resolve five conflicts by preserving main's current benchmark logic, documentation, release-site mappings and evidence exceptions, then retaining this PR's exact original ten-file payload. The runtime change and original report/capsule bytes remain unchanged. All 2,114 original evidence records were decoded and verified against their retained lengths and SHA-256 hashes.Normal commit and push hooks passed. Fresh exact-head CI and the original 32-case, three-session, nine-repeat/seven-retained archive API qualification on macOS and Linux are now pending. The new qualification will retain baseline-copy controls, all gzip counterexamples and source/build/image identities separately from the historical results. No fresh performance or merge-readiness claim is made until those results complete. #1144 remains a separate native-case symlink correctness repair; the archive-buffer benchmark does not exercise that name-matching path.
Completed fresh performance qualification
The new documentation/evidence commit
244b3d410dee0d4bc84ca16adeb3a6908784d60aretains complete remeasurement of synchronized runtime head1d2a6901c46719059e1c6eb584a0025bb344fab2against main3e546dbef0993486217eb58f3f9276dd0003651c. It changes no runtime, test or BUILD bytes after measurement. See the requalification report and its separate lossless capsule; the original report and capsule are unchanged.96fc538e897cfd6598d18a9f0a6adfeab1d272b28c330b2f4e4af2072e261e84. It includes all raw samples, sources, scripts, configuration, build/link/test receipts, fixtures, hashes, transport inventory and failed preparation logs. Executable copies remain private with hashes recorded publicly.Decision: retain #1140 for its reproduced memory-TAR benefit with these explicit costs, and merge only after final exact-head CI is green. Luna monitors readiness; no older successful run is substituted for the new head. Then refresh actual main and continue the requested sequence.
Synchronization after #1144 merged
Main advanced to
e4564c1cc12d27ba877d6dfcd3489bd1b04c29f2when #1144 merged while the prior CI run was still active. Merge commit7c20214465ce50876f53ba056009fa3a12f75d0fresolves the single CHANGELOG conflict by retaining both entries. The main advance changes only that changelog, expression-design documentation,xff/engine/run.ccand its uniform-case test; the archive implementation, diagnostic source, dependencies and build configuration used for the retained measurements did not change. The symlink fix changes filesystem-native expression matching, outside the archive API diagnostic. The prior measurements remain explicitly identified with their actual measured commit rather than relabeled as new binaries; fresh current-head CI and its benchmarks are required. Commit and push hooks passed.