diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 48168432d5..13a1ccf1c7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -24,7 +24,7 @@ repos: hooks: - id: check-added-large-files # Retain the exact lossless cross-platform performance evidence archives. - exclude: ^docs/benchmarks/((executor-copy-audit|bounded-read-ahead)-20261101|(directory-task-slots|packed-visit-snapshots|bounded-match-results|useful-match-workers|direct-evaluator-workers|derived-content-handoff|immutable-default-catalogs|prepared-diff-configuration|prepared-hash-operands|aggregate-comparison-totals|byte-shingle-representations|archive-input-buffers)-20261102|(digest-and-record-storage|caller-owned-streams|direct-source-materialization|hash-field-binding|hash-field-binding-tag|forward-cursor-skipping|raw-record-reuse|similarity-prefix-view|similarity-reference-reuse|owned-word-moves|smaller-shingle-probes|native-zip-seek-evaluation|encoded-record-reuse|threshold-aware-similarity)-20261103|(column-row-reuse|word-set-insertion|field-context-owned-results|selected-summary-output|fixed-summary-metrics|direct-summary-cells|compact-comparison-entries|comparison-category-sequence|native-name-paths|uniform-native-case|compact-traversal-metadata|eager-native-paths|native-case-experiments)-20261104|(folded-ascii-globs|bulk-directory-metadata|comparison-retention-followups|comparison-storage-followups|archive-prefix-reuse|archive-directory-range|archive-directory-allocation|archive-directory-observations|native-symlink-observations|native-metadata-followups|symlink-target-demand|native-pgo|readlink-short-target|cctz-empty-format|readlink-input-path|readlink-owned-transfer|archive-lookup-trials|archive-member-keys)-20261105|archive-directory-child-capacity-20261106|shared-stage-workers-20261107|archive-index-insertion-20261107|transactional-rg-records-20261107|immutable-zip-locator-evaluation-20261107|queued-read-cancellation-20261107|archive-btree-historical-decision-20261107|compiled-field-stat-demand-20261107|unchanged-site-payloads-20261107)\.json\.gz$ + exclude: ^docs/benchmarks/((executor-copy-audit|bounded-read-ahead)-20261101|(directory-task-slots|packed-visit-snapshots|bounded-match-results|useful-match-workers|direct-evaluator-workers|derived-content-handoff|immutable-default-catalogs|prepared-diff-configuration|prepared-hash-operands|aggregate-comparison-totals|byte-shingle-representations|archive-input-buffers)-20261102|(digest-and-record-storage|caller-owned-streams|direct-source-materialization|hash-field-binding|hash-field-binding-tag|forward-cursor-skipping|raw-record-reuse|similarity-prefix-view|similarity-reference-reuse|owned-word-moves|smaller-shingle-probes|native-zip-seek-evaluation|encoded-record-reuse|threshold-aware-similarity)-20261103|(column-row-reuse|word-set-insertion|field-context-owned-results|selected-summary-output|fixed-summary-metrics|direct-summary-cells|compact-comparison-entries|comparison-category-sequence|native-name-paths|uniform-native-case|compact-traversal-metadata|eager-native-paths|native-case-experiments)-20261104|(folded-ascii-globs|bulk-directory-metadata|comparison-retention-followups|comparison-storage-followups|archive-prefix-reuse|archive-directory-range|archive-directory-allocation|archive-directory-observations|native-symlink-observations|native-metadata-followups|symlink-target-demand|native-pgo|readlink-short-target|cctz-empty-format|readlink-input-path|readlink-owned-transfer|archive-lookup-trials|archive-member-keys)-20261105|archive-directory-child-capacity-20261106|shared-stage-workers-20261107|archive-index-insertion-20261107|transactional-rg-records-20261107|immutable-zip-locator-evaluation-20261107|queued-read-cancellation-20261107|archive-btree-historical-decision-20261107|compiled-field-stat-demand-20261107|unchanged-site-payloads-20261107|archive-read-buffer-clear-20261107|archive-read-buffer-requalification-20261108)\.json\.gz$ - id: check-merge-conflict - id: check-yaml # BINARY_TYPES below: archive containers are binary but can START with text (a phar begins with diff --git a/CHANGELOG.md b/CHANGELOG.md index 2803a97e13..028b4de7bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 0.11.0 +- Avoid redundant 64 KiB buffer clearing in legacy archive extraction. + - Preserve filesystem-native name matching for dangling and cross-volume symlinks when case sensitivity is resolved lazily. diff --git a/TODO.md b/TODO.md index 7ad7f38a41..7a0e7dab18 100644 --- a/TODO.md +++ b/TODO.md @@ -1523,3 +1523,12 @@ this as intentional accounting. Implemented in PR #1122; exact post-merge main Test run 42276661524 passed both operating systems, sanitizers, coverage and changed-file lint. The initial noisy macOS screen remains in the artifact. + +- [ ] O32: Integrate the qualified one-line legacy archive read-buffer initialization change. + Both-host TAR API gains and the repeatable Mac near-64-KiB gzip costs are explicit in + [the complete qualification](docs/benchmarks/archive-read-buffer-clear-20261107.md). + Focused release/ASan tests pass both hosts and Linux MSan passes; current-base PR checks + and main verification remain required. Do not generalize to O07 buffers or ordinary CLI speed. +- [ ] Fix the legacy ReadCompressedSingleFile API's rejection of a valid empty gzip. Retain the + unchanged-parent availability control in O32; do not turn its missing content result into + a zero-time speed ratio or silently remove it from the measured matrix. diff --git a/docs/benchmarks/archive-read-buffer-clear-20261107.json.gz b/docs/benchmarks/archive-read-buffer-clear-20261107.json.gz new file mode 100644 index 0000000000..f537c1ab90 Binary files /dev/null and b/docs/benchmarks/archive-read-buffer-clear-20261107.json.gz differ diff --git a/docs/benchmarks/archive-read-buffer-clear-20261107.md b/docs/benchmarks/archive-read-buffer-clear-20261107.md new file mode 100644 index 0000000000..8ff846405c --- /dev/null +++ b/docs/benchmarks/archive-read-buffer-clear-20261107.md @@ -0,0 +1,124 @@ +# Legacy archive read buffer initialization, 2026-11-07 + +Remove only the value initialization of the 64 KiB stack array in `ReadPositionedEntry`. +The library initializes the bytes it returns, and the caller appends exactly that count. +Legacy memory and file-backed TAR reads improve on both hosts. There is a tradeoff: +macOS single-file gzip around 64 KiB becomes about 2-3.3% slower. This is a scoped API +optimization, not a claim that ordinary CLI archive searches or every extraction gets faster. + +## Scope and correctness + +The function serves `ReadMember`, `ReadMemberOfFile` and `ReadCompressedSingleFile`. +The ordinary `MemberReadSource` streaming path is unchanged and remains a control. +No source capability, read budget, error handling, archive format, compression setting, +allocation strategy, buffer size, output limit, filesystem routing or worker policy changes. +There is no additional wrapper, branch or threshold. + +A positive library read appends only the written prefix. EOF appends nothing; a negative +read propagates the existing error. Two committed tests cover byte-exact binary payloads +at 0, 1, 1,024, 65,535, 65,536, 65,537 and 131,073 bytes, exact and too-small limits, +and large/empty/short sequential calls that must not expose an earlier stack payload. +The new tests pass against both parent and candidate on both hosts. + +Each host passes five release targets: the new buffer test, archive reader, source buffer, +filesystem and format-fixture tests. Each also passes the three focused reader/source/buffer +ASan targets. Linux additionally passes those three under the repository's MSan configuration. +These are development checks on the frozen source, not the eventual integration PR's CI. +No memory-footprint saving is claimed: the stack array still occupies 64 KiB. + +## Measurement contract + +The frozen base is main `77aa9ef9bc9475adcbdd37cf3004143a96f03060`. Parent and candidate +use the same `clang_release` configuration and Clang 23.1.4, changing only the array's +initializer. Benchmark sources, runtime sources, image hashes and system-library linkage +are retained. Diagnostic executables do not load a shared mutable xff/libarchive library. +The identical parent image is copied and measured again as an independent drift control. + +The initial 16-case memory/streaming screen has three repetitions, retaining two, per +participant on each host. It remains separate. Full qualification uses four modes and +eight payload sizes: 0, 1, 64, 1,024, 65,535, 65,536, 65,537 and 1,048,576 bytes. File-backed +TAR and single-file gzip fixtures are generated outside timing, transferred unchanged, +and hash-identical across hosts. File reads use the native host filesystem with warm-up; +they are not presented as RAM-filesystem measurements. + +Each timed API result is compared against complete independently generated content. +Timing includes that consumer comparison and result destruction. Streaming creates its +ordinary source/cursor machinery; it is not a bare decoder kernel. Three independently +ordered sessions each collect nine repetitions per case/participant and average the +fastest seven. Cases are shuffled with recorded seeds and participant order rotates. +There are no concurrent same-host builds during timing. Linux pins the whole process to +one allowed CPU; macOS is unpinned. All warm-up and timed extractions run the same oracle. + +The legacy API rejects a valid empty gzip before reaching the buffer. The first full Mac +session correctly stopped on that baseline precondition. Its initial diagnostic source +and raw errors remain. Collection then resumed only missing measurements, retaining +completed successful samples. Empty gzip is an explicit availability control on both +hosts, with nine expected errors per participant/session and no timing ratio. This is an +existing separate defect, not a zero-time improvement or silently dropped case. + +Thus full qualification retains 2,511 valid timing observations and 81 expected-error +observations per host, plus all initial-screen observations. The first diagnostic build's +const optimization-barrier issue and its corrected source are also recorded. + +## Results and costs + +For each case/session, change is the candidate's fastest-seven mean divided by the +parent's, minus one. Take the median of the three session changes per case, then the +median across that mode's cases. Positive is slower. This is an equal-case descriptive +summary, not elapsed-time weighting or a statistical significance test. Parent-repeat +controls use the same calculation and are not subtracted to manufacture a corrected win. + +| Mode | Mac candidate | Mac parent repeat | Linux candidate | Linux parent repeat | +| :---------------------- | ------------: | ----------------: | --------------: | ------------------: | +| Legacy memory TAR | -8.73% | -0.09% | -3.89% | -0.01% | +| Native file-backed TAR | -1.98% | +0.71% | -1.89% | +0.22% | +| Native single-file gzip | -2.37% | +0.04% | -0.19% | -0.18% | +| Unchanged stream path | -0.79% | -0.29% | +0.14% | +0.00% | + +Small legacy memory TAR cases improve about 12-16% on Mac and 5-6% on Linux; 1 MiB +improves less than 1%. Keep the weak Linux gzip aggregate alongside the gains: it does +not establish a meaningful general gzip speedup. + +The Mac gzip costs are real counterexamples to a universal claim: + +| Payload bytes | Candidate median | Parent repeat median | +| ------------: | ---------------: | -------------------: | +| 65,535 | +2.11% | -3.05% | +| 65,536 | +3.33% | +0.32% | +| 65,537 | +3.08% | +1.72% | + +The exact-64-KiB candidate costs +3.33%, +2.57% and +4.48% across sessions, with parent +repeat -0.39%, +3.00% and +0.32%. Its parent time is about 65 microseconds. A 1 KiB gzip +case has a +9.74% individual session despite a favorable three-session median. Do not +hide those observations in the aggregate or repeat them until favorable. + +Mac's unchanged 1 MiB stream control is particularly unstable: candidate sessions are ++1.71%, -13.77% and +8.07%; parent-repeat sessions are +51.31%, -32.10% and +4.29%. +Its median is +1.71%, versus parent repeat +4.29%. The source path is unchanged, but +that does not prove all measured costs are noise. No cache, allocator, page-fault or +instruction-layout explanation is established by this experiment. + +Select the simple change for the reproducible TAR API gains, retaining the gzip tradeoff +for review. Do not add a payload-size or metadata-based workaround without an independent +mechanism and qualification. Do not remove other buffers' initialization by analogy: +O07 already retains a one-time clear after different Linux regressions. Main benchmark +and ordinary-command speedups remain unproven for this legacy helper. + +## Retained evidence + +[The lossless capsule](archive-read-buffer-clear-20261107.json.gz) contains 2,114 hashed +records, including every raw sample, summary, output/error record, both source/image +families, plans, fixed seeds, exact native fixtures, benchmark driver, validator source, +relevant configuration and both-host focused/sanitizer logs. Diagnostic image hashes are +retained; executable copies remain private. Each record was decoded and hash-checked. +The 1,106,635-byte capsule SHA-256 is +`c86d0b03d23f71cb334ee813b6d90eb98524dfe3c6082906cf6bd3f2c1b79af7`. + +Reconstruct the base, apply the retained experimental rule, restore the relevant parent +or candidate runtime file, and build the retained benchmark with `--config=clang_release`. +The initial and full drivers have their own explicit directories and sampling policies. +For a full session run `qualify.py --directory=IMAGES --platform=macos|linux --session=1` +(and separately 2 and 3), keeping its canonical fixtures and named images. A resume uses +existing raw files rather than overwriting successful measurements. No research benchmark +source or target is added to production by this change. The public PR's current-base +hooks, coverage, sanitizer, GCC and CI clang-tidy checks remain required. diff --git a/docs/benchmarks/archive-read-buffer-requalification-20261108.json.gz b/docs/benchmarks/archive-read-buffer-requalification-20261108.json.gz new file mode 100644 index 0000000000..9f895c5342 Binary files /dev/null and b/docs/benchmarks/archive-read-buffer-requalification-20261108.json.gz differ diff --git a/docs/benchmarks/archive-read-buffer-requalification-20261108.md b/docs/benchmarks/archive-read-buffer-requalification-20261108.md new file mode 100644 index 0000000000..5b9145a18a --- /dev/null +++ b/docs/benchmarks/archive-read-buffer-requalification-20261108.md @@ -0,0 +1,124 @@ +# Legacy archive buffer requalification after #1143 + +Fresh three-session measurements on current main retain the memory-TAR benefit of #1140. +This remains a scoped legacy API optimization with slower cases. It is not a general CLI +archive-search speedup, and the new results do not replace or relabel the original observations. + +## Source and measurement identity + +- Baseline: main `3e546dbef0993486217eb58f3f9276dd0003651c`, containing merged #1143 and #1139. +- Candidate: synchronized #1140 `1d2a6901c46719059e1c6eb584a0025bb344fab2`. +- The production difference is still only removal of value initialization from the 64 KiB + `ReadPositionedEntry` array. The runtime, test, BUILD, original report and original capsule + match the previously published PR bytes exactly. +- Both hosts use the original diagnostic source and driver, byte-identical native fixtures, + `--config=clang_release`, and a verified `-flto=thin` link action. Each configured output is + copied immediately after its build; later builds cannot change a measured image. +- Three independently ordered sessions retain nine observations and average the fastest seven + per case/participant. Full content comparison and result destruction remain inside timing. + Linux pins the complete benchmark process to one allowed CPU; macOS is unpinned. +- Each host retains 2,511 timing observations and 81 expected empty-gzip errors. The unchanged + streaming path and an exact baseline-image copy remain controls. All observations, per-case + means and per-session ratios were independently recomputed from raw JSON. +- Five focused archive test targets executed and passed against both variants on each host: + buffer boundaries, archive reader, source buffer, filesystem and format fixtures. + These checks do not substitute for this PR's final exact-head CI. +- Linux transport verified all 904 files (12,725,409 bytes), including binary hashes. + Measured source files, harness, driver, rule and fixture identity match across hosts. + +## Results and limits + +Positive percentages are slower. Each cell is the median of three session ratios; the mode +summary is then the median across its cases. Controls are reported alongside results, never +subtracted to manufacture an adjusted win. These descriptive medians are not significance tests. + +| Mode | macOS candidate | macOS copy | Linux candidate | Linux copy | +| :----------------------- | --------------: | ---------: | --------------: | ---------: | +| Memory TAR | -8.47% | +0.20% | -3.78% | +0.18% | +| Native file-backed TAR | -0.31% | +0.04% | -1.17% | +0.50% | +| Native single-file gzip | -2.88% | -0.71% | -0.03% | -0.08% | +| Unchanged streaming path | +0.12% | +0.46% | -0.07% | +0.13% | + +Memory-TAR improvements persist on both hosts and exceed their copy controls. All eight +memory-TAR case medians improve. Native TAR is narrower: the macOS mode median of -0.31% +does not reproduce the original -1.98% aggregate, while Linux is -1.17%. Linux gzip remains +effectively unchanged as a mode aggregate. No ordinary-command gain is established. + +Keep the counterexamples visible: macOS gzip at 65,537 bytes is +4.99% (copy -0.83%), with +sessions -5.30%, +13.01%, +4.99%. At 65,535 bytes it is +2.50% (copy +7.85%). Linux gzip +at 65,535 bytes is +2.00% (copy +0.68%), at 65,537 bytes +0.33% (copy -0.09%), and at +1 MiB +0.91% (copy -0.39%). The original exact-64-KiB macOS slowdown does not recur here; +its -10.64% median has a -3.20% copy control and substantial session variability. + +Unchanged-path controls remain variable: macOS 1 MiB streaming is -12.03% (copy -0.71%), +while Linux 1 KiB streaming is +8.15% (copy +8.34%). Linux 64-byte native TAR is +5.62% +(copy +6.61%). Do not treat every difference as a causal buffer effect or silently discard +noisy and slower cells. No cache, allocator or instruction-placement cause is established. + +**Decision:** retain the single initialization removal for its reproduced memory-TAR gains, +with the gzip costs and narrower native-TAR result documented. Require green current-head CI +before merge. No size heuristic, buffer-size change or unrelated initialization removal is added. + +## Every measured case + +| Case / bytes | macOS candidate | macOS copy | Linux candidate | Linux copy | +| :--------------------- | --------------: | ---------: | --------------: | ---------: | +| gzip-read/1 | -4.75% | -7.70% | -6.18% | -0.09% | +| gzip-read/64 | -2.04% | +1.55% | -5.75% | +0.63% | +| gzip-read/1024 | -3.49% | -0.50% | -5.61% | +0.18% | +| gzip-read/65535 | +2.50% | +7.85% | +2.00% | +0.68% | +| gzip-read/65536 | -10.64% | -3.20% | -0.03% | -0.08% | +| gzip-read/65537 | +4.99% | -0.83% | +0.33% | -0.09% | +| gzip-read/1048576 | -2.88% | -0.71% | +0.91% | -0.39% | +| legacy-read/0 | -15.19% | +0.15% | -4.86% | +0.83% | +| legacy-read/1 | -14.02% | +0.24% | -5.53% | +0.17% | +| legacy-read/64 | -12.56% | +0.33% | -4.98% | +0.49% | +| legacy-read/1024 | -11.01% | -1.07% | -4.92% | +0.05% | +| legacy-read/65535 | -5.21% | +0.16% | -2.70% | -1.13% | +| legacy-read/65536 | -5.94% | -0.32% | -2.29% | -0.52% | +| legacy-read/65537 | -5.46% | +0.72% | -2.27% | +0.18% | +| legacy-read/1048576 | -0.16% | +1.89% | -0.69% | +1.20% | +| native-read/0 | -2.55% | +0.16% | -2.64% | +0.97% | +| native-read/1 | -3.31% | -0.71% | -2.64% | +0.80% | +| native-read/64 | -0.13% | +1.01% | +5.62% | +6.61% | +| native-read/1024 | -0.49% | +0.29% | -3.14% | +0.36% | +| native-read/65535 | -0.57% | -0.12% | -1.38% | -0.12% | +| native-read/65536 | +0.43% | +1.02% | -0.65% | +0.63% | +| native-read/65537 | -0.07% | -1.13% | -0.91% | -0.09% | +| native-read/1048576 | +0.73% | -0.07% | -0.96% | +0.21% | +| stream-control/0 | +1.23% | +0.70% | -4.01% | -3.51% | +| stream-control/1 | +1.34% | +0.30% | +0.29% | +0.58% | +| stream-control/64 | -2.42% | -1.13% | +0.87% | +7.23% | +| stream-control/1024 | +3.09% | +0.61% | +8.15% | +8.34% | +| stream-control/65535 | -3.81% | -2.01% | +0.28% | +0.11% | +| stream-control/65536 | +0.35% | +3.29% | -1.29% | -0.38% | +| stream-control/65537 | -0.11% | +1.21% | -1.89% | -1.46% | +| stream-control/1048576 | -12.03% | -0.71% | -0.41% | +0.15% | + +Empty gzip remains an expected pre-buffer API error, with no timing ratio. Memory and native +TAR fixtures include empty content. Native file-backed cases use warm ordinary filesystem +reads; they are not relabeled as memory-filesystem measurements. + +## Setup, provenance and reproduction + +The first macOS preparation stopped before timing because a guard searched raw Bazel JSON +for an equals sign that Bazel encoded as a Unicode escape. Decoding the JSON and checking +the actual link arguments corrected that guard; the successful build image/logs remain retained. +Linux preparation stopped twice before timing because its `/tmp` tmpfs user quota prevented +compiler temporary writes. Setting both client and action `TMPDIR` to campaign storage on +the main filesystem fixed preparation. No old experiments or user data were deleted. +All successful timing observations belong to the final complete runs; none were overwritten +or repeated for a favorable outcome. Neither host ran this campaign's builds during timing. + +[The new lossless capsule](archive-read-buffer-requalification-20261108.json.gz) contains 1,842 individually hashed records +and 1,008,577 compressed bytes. SHA-256: `96fc538e897cfd6598d18a9f0a6adfeab1d272b28c330b2f4e4af2072e261e84`. +It retains every raw observation, complete summaries, both driver revisions, source/configuration +snapshots, build/link/test receipts, fixture bytes, transport inventory and preparation failures. +Executable copies remain private; their sizes and SHA-256 hashes are retained in the capsule. +The original 2026-11-07 report and capsule remain unchanged. + +Rebuild the synchronized source with the retained private benchmark rule/source, substituting +only the recorded baseline or candidate runtime file. Use `requalify_archive.py` for build, +copy, ThinLTO verification, focused tests and three sessions; its Linux variant additionally +routes build temporaries. `analyze_archive.py` independently recomputes all raw means and +ratios. Neither private diagnostic source nor rule is added to production. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index 65f1b7719d..47dc668253 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -5000,3 +5000,31 @@ identity-stable random access, visible-local-member association and shared read requirements. The separate deferred-bzip2 content bug is tracked. All 19 source/configuration/log records are retained losslessly; no runtime implementation, speedup, full-suite/sanitizer qualification or public-head CI claim is made. O19b3 and streamed-size correctness remain open. + +### O32: legacy archive extraction buffer initialization + +[The complete two-host qualification](benchmarks/archive-read-buffer-clear-20261107.md) +changes only the 64 KiB stack array's value initialization in `ReadPositionedEntry`. +The library-written prefix and existing error/limit handling remain unchanged. Three +independently ordered sessions of nine repetitions retaining seven per case show legacy +memory-TAR medians -8.73% Mac/-3.89% Linux and native-TAR -1.98%/-1.89%, including +complete output consumption. The unchanged streaming path is a separate control; no +ordinary-command or current main benchmark speedup is established. + +Keep the tradeoff prominent: Mac gzip at 65,535/65,536/65,537 bytes costs +2.11/+3.33/+3.08% +per-case medians; the exact-64-KiB case is slower in every session. The weak Linux gzip +aggregate (-0.19%) is not a general speedup. Full raw controls include noisy Mac stream +and gzip observations; no cache/allocator explanation is claimed and no heuristic is added. +The simpler initialization change is proposed for the reproducible TAR API gains with +those explicit costs. Retain O07's separate one-time-clear decision. + +All five focused release targets pass each host, three focused ASan targets pass each, +and three Linux MSan targets pass. The new byte-boundary tests also pass on the parent. +A valid empty gzip is rejected by the unchanged legacy API before this buffer; its nine +expected errors per participant/session remain availability data with no speed ratio. +That separate defect is tracked. All 2,114 hashed evidence records are retained; no +research benchmark target/source is published. Current public integration CI remains required. + +[Fresh post-#1143 requalification](benchmarks/archive-read-buffer-requalification-20261108.md) retains the memory-TAR +benefit (-8.47% macOS/-3.78% Linux) with narrower native-TAR gains and every slower gzip +case and copy control. The original report/capsule stays frozen; exact-head CI remains required. diff --git a/extra_modules/archive/BUILD.bazel b/extra_modules/archive/BUILD.bazel index fbfac8169f..39402c4b43 100644 --- a/extra_modules/archive/BUILD.bazel +++ b/extra_modules/archive/BUILD.bazel @@ -735,3 +735,18 @@ cc_binary( "@zlib", ], ) + +cc_test( + name = "archive_read_buffer_test", + size = "small", + srcs = ["archive_read_buffer_test.cc"], + deps = [ + ":archive_reader_cc", + ":memory_archive_test_cc", + "@abseil-cpp//absl/status", + "@googletest//:gtest", + "@googletest//:gtest_main", + "@mboworks_mbo//mbo/testing:matchers_cc", + "@mboworks_mbo//mbo/testing:status_cc", + ], +) diff --git a/extra_modules/archive/archive_read_buffer_test.cc b/extra_modules/archive/archive_read_buffer_test.cc new file mode 100644 index 0000000000..d6ccf48160 --- /dev/null +++ b/extra_modules/archive/archive_read_buffer_test.cc @@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include + +#include "absl/status/status.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "mbo/testing/matchers.h" +#include "mbo/testing/status.h" +#include "xff/archive/archive_reader.h" +#include "xff/archive/memory_archive_test.h" + +namespace xff::archive { +namespace { +using ::mbo::testing::EqualsText; +using ::mbo::testing::IsOkAndHolds; +using ::mbo::testing::StatusIs; + +struct ArchiveReadBufferTest : ::testing::Test {}; + +TEST_F(ArchiveReadBufferTest, ReturnsOnlyWrittenBytesForEmptyPartialAndMultipleBlocks) { + constexpr auto kSizes = std::to_array({0, 1, 1'024, 65'535, 65'536, 65'537, 131'073}); + for (const auto size : kSizes) { + SCOPED_TRACE(size); + std::string content(size, 'x'); + for (std::size_t index = 0; index < size; ++index) { + content.at(index) = static_cast(index % 251); + } + const auto members = std::to_array({ + { + .path = "data.bin", + .content = content, + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto bytes, MakeMemoryTar(members)); + EXPECT_THAT(ReadMember(bytes, "data.bin"), IsOkAndHolds(EqualsText(content))); + EXPECT_THAT(ReadMember(bytes, "data.bin", size), IsOkAndHolds(EqualsText(content))); + if (size > 1) { + EXPECT_THAT(ReadMember(bytes, "data.bin", size - 1), StatusIs(absl::StatusCode::kResourceExhausted)); + } + } +} + +TEST_F(ArchiveReadBufferTest, EmptyAndShortReadsDoNotExposeAnEarlierLargePayload) { + const std::string large(131'073, 'x'); + const auto members = std::to_array({ + { + .path = "large.txt", + .content = large, + }, + { + .path = "empty.txt", + .content = "", + }, + { + .path = "small.txt", + .content = "short", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto bytes, MakeMemoryTar(members)); + EXPECT_THAT(ReadMember(bytes, "large.txt"), IsOkAndHolds(EqualsText(large))); + EXPECT_THAT(ReadMember(bytes, "empty.txt"), IsOkAndHolds(EqualsText(""))); + EXPECT_THAT(ReadMember(bytes, "small.txt"), IsOkAndHolds(EqualsText("short"))); +} + +} // namespace +} // namespace xff::archive diff --git a/extra_modules/archive/archive_reader.cc b/extra_modules/archive/archive_reader.cc index 933c75f860..eddae422af 100644 --- a/extra_modules/archive/archive_reader.cc +++ b/extra_modules/archive/archive_reader.cc @@ -160,7 +160,7 @@ absl::StatusOr ReadPositionedEntry( const auto reserve = static_cast(hint); contents.reserve(max_bytes != 0 ? std::min(reserve, max_bytes) : reserve); } - std::array buffer{}; + std::array buffer; while (true) { const ::ssize_t read = ::archive_read_data(handle, buffer.data(), buffer.size()); if (read == 0) { diff --git a/release-site.json b/release-site.json index 58fc2101e4..a9c7fb7b29 100644 --- a/release-site.json +++ b/release-site.json @@ -147,7 +147,9 @@ "docs/benchmark-extended-history-campaign.md": "docs/benchmark-extended-history-campaign.html", "docs/benchmarks/find-listing-optimization-20261107.md": "docs/benchmarks/find-listing-optimization-20261107.html", "docs/benchmarks/lazy-content-state-20261107.md": "docs/benchmarks/lazy-content-state-20261107.html", - "docs/benchmarks/deferred-zip-size-investigation-20261107.md": "docs/benchmarks/deferred-zip-size-investigation-20261107.html" + "docs/benchmarks/deferred-zip-size-investigation-20261107.md": "docs/benchmarks/deferred-zip-size-investigation-20261107.html", + "docs/benchmarks/archive-read-buffer-clear-20261107.md": "docs/benchmarks/archive-read-buffer-clear-20261107.html", + "docs/benchmarks/archive-read-buffer-requalification-20261108.md": "docs/benchmarks/archive-read-buffer-requalification-20261108.html" }, "generated_html": { "XFF.md": { @@ -186,6 +188,8 @@ "docs/benchmarks/extended-history-viewer-check-20261107.json": "docs/benchmarks/extended-history-viewer-check-20261107.json", "docs/benchmarks/find-listing-optimization-20261107.json": "docs/benchmarks/find-listing-optimization-20261107.json", "docs/benchmarks/lazy-content-state-20261107.json": "docs/benchmarks/lazy-content-state-20261107.json", - "docs/benchmarks/deferred-zip-size-investigation-20261107.json.gz": "docs/benchmarks/deferred-zip-size-investigation-20261107.json.gz" + "docs/benchmarks/deferred-zip-size-investigation-20261107.json.gz": "docs/benchmarks/deferred-zip-size-investigation-20261107.json.gz", + "docs/benchmarks/archive-read-buffer-clear-20261107.json.gz": "docs/benchmarks/archive-read-buffer-clear-20261107.json.gz", + "docs/benchmarks/archive-read-buffer-requalification-20261108.json.gz": "docs/benchmarks/archive-read-buffer-requalification-20261108.json.gz" } }