diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4993552d84..f817fffbba 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|archive-read-buffer-clear-20261107|archive-read-buffer-requalification-20261108|language-list-reserve-20261107|language-list-reserve-requalification-20261108|ordered-output-credit-20261107|adaptive-ordered-output-20261109|direct-line-scan-spans-20261109-(linux|macos))\.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|language-list-reserve-20261107|language-list-reserve-requalification-20261108|ordered-output-credit-20261107|adaptive-ordered-output-20261109|(direct-line-scan-spans|prepared-member-name)-20261109-(linux|macos))\.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/TODO.md b/TODO.md index f4be59b404..a7b37c58dd 100644 --- a/TODO.md +++ b/TODO.md @@ -585,6 +585,10 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms +27% on Mac and +11% on Linux. Eliminate those costs without arbitrary count/size thresholds, preserving identity, budgets, duplicate semantics, exact-content oracles and the rejected root-offset variant. Native source identity and streamed-ZIP sizes remain separate open work. +- [ ] O19b4: Qualify reuse of validated plain-TAR payload locators independently of name + preparation. The 10,000-member native search still spends roughly 8-13 seconds replaying + headers. Preserve format validation, exact source identity, duplicate selection, nested and + sequential-source fallbacks, and shared read budgets; do not bypass decoder checks. - [ ] O19b: Qualify archive seeking/index/replay and decoder admission independently. Investigate native/memory ZIP seeking before concurrency; preserve sequential-source and shared-budget fallbacks. - [x] O19c: Qualify caller-owned reads for immutable memory and archive-member cursors, including @@ -611,13 +615,11 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms separate hardware diagnostics are retained in [the allocation decisions](docs/benchmarks/archive-directory-allocation-20261105.md); microbenchmark gains alone do not qualify either runtime implementation. -- [ ] O19g3: Prepare the requested member's normalized name once per streamed open, - preserving null-header, normalization, error and duplicate-selection behavior. The - current `MemberSource::Open` repeats `NormalizeMemberName(member_)` while walking - preceding headers. Retain cold/first-member/missing-member controls and measure on - both hosts before adoption. The scanner qualification also exposes roughly eight - seconds for the broad 10,000-member, single-worker TAR invocation; reuse observed plain-TAR payload locators - as a separate, format-validated O19b experiment rather than bypass decoder checks. +- [x] O19g3: Qualify once-per-open requested member-name normalization, preserving nullable + C pathnames, normalization, duplicate/nonregular selection, source ownership and errors. + [Both-host measurements and complete data](docs/benchmarks/prepared-member-name-20261109.md) + retain first/missing/reopened/fresh-wrapper controls, slower results, and the Linux + missing-member microbenchmark tradeoff; no universal speedup is claimed. - [x] O19i: Bound initial archive-directory result storage to reduce small-listing vector reallocations, retaining tiny/root/chain costs and complete native controls. Full both-host qualification and the held O19g variants are in @@ -1589,3 +1591,11 @@ this as intentional accounting. harness versions, slower cases and all raw observations in [the decision report](docs/benchmarks/language-list-reserve-20261107.md). Generated immutable catalog representation remains a separate O14 candidate. + +### Shared local measurement ownership + +- [ ] Coordinate performance measurements and compiler work across simultaneous worktrees/tasks + with an explicit host-level lease. Per-campaign locks alone do not prevent contamination; + retain contention records, wait for foreign work rather than terminating it, and qualify + each affected cohort only after an uncontended rerun. The member-name report records a + concrete benchmark overlap and an independently started MSan compiler. diff --git a/docs/benchmarks/prepared-member-name-20261109-linux.json.gz b/docs/benchmarks/prepared-member-name-20261109-linux.json.gz new file mode 100644 index 0000000000..271ddab6a2 Binary files /dev/null and b/docs/benchmarks/prepared-member-name-20261109-linux.json.gz differ diff --git a/docs/benchmarks/prepared-member-name-20261109-macos.json.gz b/docs/benchmarks/prepared-member-name-20261109-macos.json.gz new file mode 100644 index 0000000000..8be29b452b Binary files /dev/null and b/docs/benchmarks/prepared-member-name-20261109-macos.json.gz differ diff --git a/docs/benchmarks/prepared-member-name-20261109.md b/docs/benchmarks/prepared-member-name-20261109.md new file mode 100644 index 0000000000..8b52cf11f1 --- /dev/null +++ b/docs/benchmarks/prepared-member-name-20261109.md @@ -0,0 +1,190 @@ +# Prepared streamed archive member names + +Prepare the requested member's normalized name once for each streamed open. Keep the stored +header name normalization inside the header loop. The requested view borrows the source object's +owned string for the duration of that open; it introduces no allocation, lock, cache, or lifetime +change. + +A small value-returning adapter contains libarchive's nullable pathname API and serves both +streaming lookup paths. An absent pathname does not match even an empty request. An empty, +present pathname may match an empty normalized request. The first matching header still wins; +a matching directory or symlink remains an error rather than falling through to a later regular +duplicate. Normalization still strips leading `./` components and trailing slashes (retaining a single `/`), without +resolving interior `..` components or folding case. + +The candidate reduces normal streamed-read elapsed time modestly on both hosts. Keep the +Linux missing-member microbenchmark cost (up to 3.45%) as an explicit tradeoff; this is not a +universal speedup. It adds no capability or command-line setting. + +## Results + +Negative changes mean faster. The copy is an independently stored, byte-identical main image; +it is a diagnostic participant, not a divisor or correction factor. Every native case requests +one worker. These are retained means, not confidence bounds or a weighted deployment workload. + +| Host | Task | Tree | Members/files | Main ms | Candidate | Copy | +| ----- | ------------- | ----- | ------------: | --------: | --------: | -----: | +| macos | archive-lines | broad | 100 | 12.706 | -2.27% | +0.60% | +| macos | archive-lines | broad | 1,000 | 125.983 | +1.42% | +1.79% | +| macos | archive-lines | broad | 10,000 | 10283.505 | -2.35% | -3.89% | +| macos | archive-lines | deep | 100 | 9.471 | +0.62% | -0.66% | +| macos | archive-lines | deep | 1,000 | 144.903 | -1.21% | -1.62% | +| macos | archive-lines | deep | 10,000 | 13273.324 | -2.09% | -2.44% | +| macos | files-exact | broad | 100 | 4.940 | -2.59% | -2.40% | +| macos | files-exact | broad | 100,000 | 25.316 | -0.49% | +0.49% | +| macos | files-exact | deep | 100 | 5.895 | -0.74% | +1.34% | +| macos | files-exact | deep | 100,000 | 30.402 | -0.58% | +1.26% | +| linux | archive-lines | broad | 100 | 2.022 | -0.67% | -1.03% | +| linux | archive-lines | broad | 1,000 | 87.058 | +0.23% | +2.00% | +| linux | archive-lines | broad | 10,000 | 8277.677 | -0.64% | +0.44% | +| linux | archive-lines | deep | 100 | 2.105 | -0.32% | -0.77% | +| linux | archive-lines | deep | 1,000 | 122.435 | -1.48% | -0.01% | +| linux | archive-lines | deep | 10,000 | 12554.980 | -1.53% | -0.13% | +| linux | files-exact | broad | 100 | 0.841 | -0.34% | -3.45% | +| linux | files-exact | broad | 100,000 | 35.723 | -0.07% | -0.40% | +| linux | files-exact | deep | 100 | 0.917 | +0.37% | -3.12% | +| linux | files-exact | deep | 100,000 | 43.738 | -0.04% | -0.53% | + +Descriptive geometric ratios across the original kernel cases, with each case weighted +equally (this does not describe an expected command mix): + +| Host | Kernel | Cases | Candidate | Copy | +| ----- | ------ | ----: | --------: | -----: | +| macos | last | 24 | -3.32% | -2.55% | +| macos | list | 24 | -2.29% | +1.55% | +| linux | last | 24 | -4.36% | -0.25% | +| linux | list | 24 | -4.48% | -1.11% | + +First/missing controls use a fresh archive cursor in every operation, and distinguish +reusing a member wrapper from constructing a new wrapper. Each row groups four cases: +two archive sizes and two requested-name spellings. + +| Host | Control | Candidate | Copy | +| ----- | --------------------- | --------: | -----: | +| macos | first/reopen | -0.39% | -0.19% | +| macos | first/fresh-wrapper | -1.09% | -0.03% | +| macos | missing/reopen | -2.70% | -0.80% | +| macos | missing/fresh-wrapper | -2.49% | -0.95% | +| linux | first/reopen | -0.06% | -0.32% | +| linux | first/fresh-wrapper | +0.08% | -0.69% | +| linux | missing/reopen | +2.60% | -0.89% | +| linux | missing/fresh-wrapper | +2.54% | -0.90% | + +### Slower observations and limits + +The original Mac legacy last-member cases at 10/128 and 1,000/128 reported +6.74% and ++5.63%. A fixed confirmation selected every candidate above +5% or copy outside +/-5% +(16 cases), before examining its results. The same two cases then measured -11.60% +and +0.38%. The original nested-memory listing at 10/4,096 reported +17.09% with a ++21.39% copy control; its repeat measured -11.92%. The repeat retains an unrelated +legacy listing cost of +2.35%. Neither cohort replaces the other, and the large copy +movements do not establish a particular hardware, placement, or image-backing cause. + +Linux missing-member controls retain descriptive group costs of +2.60% and +2.54%, +with a worst individual point of +3.45%. This error-path tradeoff accompanies faster +normal last-member kernels and modest native archive improvements. The largest native +point regression is Mac broad/1,000 members at +1.42% (copy +1.79%). Ordinary native +listings remain near parity. Small gains and moving controls limit causal confidence; +retain the simpler invariant preparation without promising larger end-to-end gains. + +### Shipping image sizes + +These are actual configured executables before stripping or compression, not tagged +release assets. The manual benchmark overlay is not linked into the CLI. + +| Host | Main bytes | Candidate bytes | Change | +| ----- | ---------: | --------------: | -----: | +| macos | 8,322,520 | 8,322,520 | +0 | +| linux | 18,578,064 | 18,578,184 | +120 | + +## Data + +Download the [Mac data](prepared-member-name-20261109-macos.json.gz) and +[Linux data](prepared-member-name-20261109-linux.json.gz). They retain all original +iteration documents, native samples, source/build identities, helper overlays, scoped +verification logs, rejected/contended attempts, and estimator checks. JSON documents are +semantically complete, with original byte lengths and SHA-256 hashes; exact raw bytes +and frozen images remain in the retained evidence directories. + +| Dataset | Compressed bytes | SHA-256 | +| ------- | ---------------: | ---------------------------------------------------------------- | +| macos | 847872 | 91fa836dc223c0640b50e1551cd21def14e7556bb80ccd2625d163079d3ab674 | +| linux | 1477789 | 08f7c48fb5c63cb212f41dd71a47a0104b8a6e926961749e165ebf5a43655113 | + +## Source and build identities + +The shipping baseline is main `91437fef41f86c7f048dd542e77f21103d4e1268`; the runtime candidate +is `b961a4bf977691f8e76e31b7dcf582cede6596f7`. Both were built from the same owned workspace on +each host, using the actual `clang_release` and `xff_full` recipes with ThinLTO. Each configured +executable was queried and frozen immediately. Identical-copy controls preserve the `xff_full` +invocation name, use ordinary independent copies, and appear once per rotated round. + +The candidate's initial signed development commit was +`c041358fdb6cbf42849f1d88cee2c67af44bf23e`. Its complete Git tree and the rebased runtime tree are +identical (`fac59eeb91a4cd9236c43fbb6dc0cd102bf653d0`); the rebase changed ancestry after the prior +PR was squash-merged. This equivalence supports development evidence only, never checks from a +different PR context. + +Mac main's full file hash differs from the previous scanner report's image because cached +Mach-O debugger source-directory strings contain different sandbox paths. All eleven file-backed +sections and symbol values match after normalizing only those debugger directory prefixes. +The current experiment uses its own baseline consistently; it makes no whole-file identity claim +against that older Mac image. The Linux baseline matches the previous shipping image byte for byte. + +First/missing-member controls add an identical benchmark-only translation-unit overlay to both +runtime revisions. Every runtime source and build rule remains the corresponding signed revision's +content. These helpers are separate from the shipping CLI images; their source, patch, build +commands, and hashes are retained explicitly. + +## Measurement contract + +All timings use unsanitized optimized binaries. The native CLI corpus resides on case-sensitive +HFS+ RAM storage on Mac and verified tmpfs on Linux. Each measured CLI invocation runs as a fresh +process. The memory fixtures are resident; none of these results claims cold operating-system +cache performance. Linux uses an explicit single-CPU affinity for the native controls. On Mac, +`--jobs=1` requests one worker; no CPU pinning is claimed. + +Native cases cover broad and deep TAR trees with 100, 1,000, and 10,000 members, plus ordinary +listings of 100 and 100,000 files as unrelated controls. Each image receives one discarded +warmup and nine measured rounds per case; retain the fastest seven for its point estimate. +Every invocation has an independent path/content oracle. Native timestamps exclude Python's +process-launch overhead. All slower observations and output-validation counts remain in the data. + +The unchanged archive-source kernel covers 48 cases: four source implementations, listing or +last-member reading, three member counts, and two payload sizes. First/missing controls add 16 +cases spanning ten or ten thousand headers, normalized or prefixed request spellings, and reused +or freshly constructed member wrappers. Both kernel groups rotate main, candidate, and an +identical-copy control across three blocks. Each case uses nine repetitions per block, retains +seven per block, and averages those three retained means. Fixture construction is outside timing; +read contents or expected missing errors are checked within every measured operation. + +An earlier Linux kernel cohort overlapped a newly started independent benchmark. Its raw results +remain preserved as contended development evidence. The full fixed matrix was repeated after that +campaign completed, with before/after checks for competing measurement/compiler work. The first +native attempt stopped with zero timings when another task started an MSan compilation. A fresh +native cohort waits for sixty seconds clear before starting, with additional checks between +rotated rounds. No result is divided by the copy-control ratio or silently replaced because it is +slower. These checks do not claim an otherwise physically isolated machine. + +## Correctness and validation + +Seven committed member-source tests cover normalized requests, first duplicate selection, +directory/symlink rejection, missing/dot/case behavior, source ownership and reopening, invalid +source errors, and the actual C boundary's absent and present pathnames. The shared adapter's +short-circuit guard is exercised on all four branches in instrumented library code; it is not an +uninstrumented inline test substitute. + +At the signed development runtime source, the full configured release suite passes 345 targets +on each host, allowing Bazel reuse only for unchanged declared inputs. Eight affected suites pass +freshly under ASan and TSan on both hosts and MSan on Linux. Actual affected translation-unit +clang-tidy passes on both hosts. The additional benchmark translation unit receives its own check. +These are scoped development results; final pushed-head checks belong to the resulting PR and +must complete in its own context. + +## Retained work + +This changes invariant name preparation only. It does not add a TAR index, avoid archive replay, +or change decoder admission, safety policy, metadata observation, concurrency, or output ordering. +The larger O19b opportunity remains: reuse validated plain-TAR payload locators independently, +while preserving format checks and read budgets. The broader O19g borrowed-key experiments and +all remaining performance candidates retain their separate TODO entries. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index c57ecc26e3..d778375c12 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -5233,3 +5233,11 @@ contracts. Both-host native, engine, legacy/direct kernel and independent-image qualification select stack storage, retaining the heap variant and every slower observation. Full implementation-head release and affected sanitizer suites pass; the report/data commit receives its own current-head checks. Broader O18 remains open. + +### Prepared streamed archive member names + +The [member-name qualification](benchmarks/prepared-member-name-20261109.md) prepares a requested +name once per streamed open. Preserve nullable C names, ownership, duplicate selection and errors. +Both-host shipping CLI, last-member/listing and first/missing controls retain slower results, +including the Linux missing-member tradeoff. Plain-TAR payload locators and host-wide measurement +coordination remain separate open tasks; broader O19 work is not complete. diff --git a/extra_modules/archive/BUILD.bazel b/extra_modules/archive/BUILD.bazel index 39402c4b43..54d13b0fe0 100644 --- a/extra_modules/archive/BUILD.bazel +++ b/extra_modules/archive/BUILD.bazel @@ -750,3 +750,22 @@ cc_test( "@mboworks_mbo//mbo/testing:status_cc", ], ) + +cc_test( + name = "archive_member_source_test", + size = "small", + srcs = ["archive_member_source_test.cc"], + deps = [ + ":archive_reader_cc", + ":memory_archive_test_cc", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@googletest//:gtest", + "@googletest//:gtest_main", + "@libarchive//libarchive", + "@mboworks_mbo//mbo/status:status_macros_cc", + "@mboworks_mbo//mbo/testing:matchers_cc", + "@mboworks_mbo//mbo/testing:status_cc", + "@xff_extras_api//:read_source_cc", + ], +) diff --git a/extra_modules/archive/archive_member_source_test.cc b/extra_modules/archive/archive_member_source_test.cc new file mode 100644 index 0000000000..70df2d30d5 --- /dev/null +++ b/extra_modules/archive/archive_member_source_test.cc @@ -0,0 +1,162 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include + +#include +#include +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "mbo/status/status_macros.h" +#include "mbo/testing/matchers.h" +#include "mbo/testing/status.h" +#include "xff/archive/archive_reader.h" +#include "xff/archive/archive_reader_internal.h" +#include "xff/archive/memory_archive_test.h" +#include "xff/vfs/read_source.h" + +namespace xff::archive { +namespace { +using ::mbo::testing::EqualsText; +using ::mbo::testing::IsOkAndHolds; +using ::mbo::testing::StatusIs; +using ::testing::IsFalse; +using ::testing::IsTrue; +using ::testing::NotNull; + +struct ArchiveMemberSourceTest : ::testing::Test { + static absl::StatusOr Source(std::span members) { + MBO_ASSIGN_OR_RETURN(auto bytes, MakeMemoryTar(members)); + return vfs::MemoryReadSource(std::move(bytes)); + } +}; + +TEST_F(ArchiveMemberSourceTest, EntryNamesHandleAbsentDifferentAndNormalizedValues) { + // XFF_ABI_POINTER: the test owns a C entry and releases it through libarchive. + const std::unique_ptr entry( + ::archive_entry_new(), &::archive_entry_free); + ASSERT_THAT(entry, NotNull()); + EXPECT_THAT(archive_reader_internal::EntryMatchesNormalizedName(*entry, "dir/value"), IsFalse()); + EXPECT_THAT(archive_reader_internal::EntryMatchesNormalizedName(*entry, ""), IsFalse()); + ::archive_entry_set_pathname(entry.get(), ""); + EXPECT_THAT(archive_reader_internal::EntryMatchesNormalizedName(*entry, ""), IsTrue()); + ::archive_entry_set_pathname(entry.get(), "././dir/value///"); + EXPECT_THAT(archive_reader_internal::EntryMatchesNormalizedName(*entry, "dir/value"), IsTrue()); + EXPECT_THAT(archive_reader_internal::EntryMatchesNormalizedName(*entry, "dir/other"), IsFalse()); + // The adapter must not silently normalize a second copy of the requested name. + EXPECT_THAT(archive_reader_internal::EntryMatchesNormalizedName(*entry, "./dir/value"), IsFalse()); +} + +TEST_F(ArchiveMemberSourceTest, NormalizedRequestsKeepTheFirstMatchingHeader) { + const auto members = std::to_array({ + { + .path = "unrelated.txt", + }, + { + .path = "dir/value-other", + }, + { + .path = "././dir/value", + .content = "first\n", + }, + { + .path = "dir/value", + .content = "second\n", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto source, Source(members)); + static constexpr auto kRequests = std::to_array({ + "dir/value", + "./dir/value/", + "././dir/value///", + }); + for (const auto request : kRequests) { + SCOPED_TRACE(request); + const auto member = MemberReadSource(source, std::string(request)); + EXPECT_THAT(vfs::ReadSourceBytes(*member, 1'024), IsOkAndHolds(EqualsText("first\n"))); + } +} + +TEST_F(ArchiveMemberSourceTest, ADirectoryMatchDoesNotFallThroughToARegularDuplicate) { + const auto members = std::to_array({ + { + .path = "./blocked/", + .directory = true, + }, + { + .path = "blocked", + .content = "must not be returned", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto source, Source(members)); + const auto member = MemberReadSource(source, "././blocked///"); + EXPECT_THAT(vfs::ReadSourceBytes(*member, 1'024), StatusIs(absl::StatusCode::kFailedPrecondition)); +} + +TEST_F(ArchiveMemberSourceTest, ASymlinkMatchDoesNotFallThroughToARegularDuplicate) { + const auto members = std::to_array({ + { + .path = "./link", + .symlink_target = "elsewhere", + }, + { + .path = "link", + .content = "must not be returned", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto source, Source(members)); + const auto member = MemberReadSource(source, "././link///"); + EXPECT_THAT(vfs::ReadSourceBytes(*member, 1'024), StatusIs(absl::StatusCode::kFailedPrecondition)); +} + +TEST_F(ArchiveMemberSourceTest, MissingNamesDoNotResolveDotSegmentsOrFoldCase) { + const auto members = std::to_array({ + { + .path = "dir/value", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto source, Source(members)); + static constexpr auto kRequests = std::to_array({ + "dir/missing", + "dir/../dir/value", + "Dir/value", + }); + for (const auto request : kRequests) { + SCOPED_TRACE(request); + const auto member = MemberReadSource(source, std::string(request)); + EXPECT_THAT(vfs::ReadSourceBytes(*member, 1'024), StatusIs(absl::StatusCode::kNotFound)); + } +} + +TEST_F(ArchiveMemberSourceTest, ReopenedSourcesOwnTheirOriginalRequestedName) { + const auto members = std::to_array({ + { + .path = "dir/value", + .content = "retained\n", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto source, Source(members)); + std::string request = "././dir/value///"; + const auto member = MemberReadSource(source, request); + request.assign(1'024, 'x'); + for (const auto repeat : {0, 1}) { + SCOPED_TRACE(repeat); + EXPECT_THAT(vfs::ReadSourceBytes(*member, 1'024), IsOkAndHolds(EqualsText("retained\n"))); + } +} + +TEST_F(ArchiveMemberSourceTest, InvalidSourceRemainsAnOpenError) { + const auto source = vfs::MemoryReadSource("not an archive"); + const auto member = MemberReadSource(source, "missing"); + EXPECT_THAT(vfs::ReadSourceBytes(*member, 1'024), StatusIs(absl::StatusCode::kInvalidArgument)); +} + +} // namespace +} // namespace xff::archive diff --git a/extra_modules/archive/archive_reader.cc b/extra_modules/archive/archive_reader.cc index eddae422af..67924228a4 100644 --- a/extra_modules/archive/archive_reader.cc +++ b/extra_modules/archive/archive_reader.cc @@ -40,6 +40,13 @@ #include "xff/archive/member_path.h" namespace xff::archive { + +bool archive_reader_internal::EntryMatchesNormalizedName(struct ::archive_entry& entry, std::string_view wanted) { + // XFF_ABI_POINTER: libarchive returns a borrowed pathname, possibly absent. + const char* stored = ::archive_entry_pathname(&entry); + return stored != nullptr && NormalizeMemberName(stored) == wanted; +} + namespace { // The streaming block size for both the header walk and the content read: one value, so the two @@ -192,8 +199,7 @@ absl::StatusOr ReadMemberOfOpened( if (status != ARCHIVE_OK && status != ARCHIVE_WARN) { return absl::DataLossError(absl::StrCat("archive read failed: ", LastError(handle))); } - const char* const stored = ::archive_entry_pathname(entry); - if (stored == nullptr || NormalizeMemberName(stored) != wanted) { + if (!archive_reader_internal::EntryMatchesNormalizedName(*entry, wanted)) { continue; // not this one; libarchive skips its data on the next header read } return ReadPositionedEntry(handle_ref, *entry, member, max_bytes); @@ -385,6 +391,7 @@ class MemberSource final : public vfs::ReadSource { absl::StatusOr> Open() const override { MBO_ASSIGN_OR_RETURN(auto session, OpenSourceSession(source_)); + const std::string_view wanted = NormalizeMemberName(member_); // XFF_ABI_POINTER: archive_read_next_header returns a borrowed C entry. struct ::archive_entry* entry = nullptr; while (true) { @@ -396,9 +403,7 @@ class MemberSource final : public vfs::ReadSource { MBO_RETURN_IF_ERROR(session->error); return absl::DataLossError(LastError(session->handle.get())); } - // XFF_ABI_POINTER: archive_entry_pathname returns a borrowed NUL-terminated name. - const char* stored = ::archive_entry_pathname(entry); - if (stored == nullptr || NormalizeMemberName(stored) != NormalizeMemberName(member_)) { + if (!archive_reader_internal::EntryMatchesNormalizedName(*entry, wanted)) { continue; } if (::archive_entry_filetype(entry) != AE_IFREG) { diff --git a/extra_modules/archive/archive_reader_internal.h b/extra_modules/archive/archive_reader_internal.h index 8eab73bdad..3597cd7b8f 100644 --- a/extra_modules/archive/archive_reader_internal.h +++ b/extra_modules/archive/archive_reader_internal.h @@ -5,6 +5,7 @@ #define XFF_ARCHIVE_ARCHIVE_READER_INTERNAL_H_ #include +#include #include #include @@ -28,4 +29,12 @@ absl::StatusOr ReadCompressedSingleFileWithFilterEnabler( } // namespace xff::archive::internal +namespace xff::archive::archive_reader_internal { + +// The requested name is already normalized once by its caller. Keep the C API's +// nullable borrowed pathname inside this adapter; no borrowed pointer escapes. +[[nodiscard]] bool EntryMatchesNormalizedName(struct ::archive_entry& entry, std::string_view wanted); + +} // namespace xff::archive::archive_reader_internal + #endif // XFF_ARCHIVE_ARCHIVE_READER_INTERNAL_H_ diff --git a/release-site.json b/release-site.json index 4e3bebecad..8710b89b82 100644 --- a/release-site.json +++ b/release-site.json @@ -161,7 +161,8 @@ "docs/benchmarks/adaptive-ordered-output-20261109.md": "docs/benchmarks/adaptive-ordered-output-20261109.html", "docs/benchmarks/prepared-predicate-guards-20261109.md": "docs/benchmarks/prepared-predicate-guards-20261109.html", "docs/benchmarks/bounded-stdout-buffer-20261109.md": "docs/benchmarks/bounded-stdout-buffer-20261109.html", - "docs/benchmarks/direct-line-scan-spans-20261109.md": "docs/benchmarks/direct-line-scan-spans-20261109.html" + "docs/benchmarks/direct-line-scan-spans-20261109.md": "docs/benchmarks/direct-line-scan-spans-20261109.html", + "docs/benchmarks/prepared-member-name-20261109.md": "docs/benchmarks/prepared-member-name-20261109.html" }, "generated_html": { "XFF.md": { @@ -213,6 +214,8 @@ "docs/benchmarks/prepared-predicate-guards-20261109.json.gz": "docs/benchmarks/prepared-predicate-guards-20261109.json.gz", "docs/benchmarks/bounded-stdout-buffer-20261109.json.gz": "docs/benchmarks/bounded-stdout-buffer-20261109.json.gz", "docs/benchmarks/direct-line-scan-spans-20261109-linux.json.gz": "docs/benchmarks/direct-line-scan-spans-20261109-linux.json.gz", - "docs/benchmarks/direct-line-scan-spans-20261109-macos.json.gz": "docs/benchmarks/direct-line-scan-spans-20261109-macos.json.gz" + "docs/benchmarks/direct-line-scan-spans-20261109-macos.json.gz": "docs/benchmarks/direct-line-scan-spans-20261109-macos.json.gz", + "docs/benchmarks/prepared-member-name-20261109-linux.json.gz": "docs/benchmarks/prepared-member-name-20261109-linux.json.gz", + "docs/benchmarks/prepared-member-name-20261109-macos.json.gz": "docs/benchmarks/prepared-member-name-20261109-macos.json.gz" } } diff --git a/tools/archive_source_benchmark.cc b/tools/archive_source_benchmark.cc index 87dec8af8b..3afeb58195 100644 --- a/tools/archive_source_benchmark.cc +++ b/tools/archive_source_benchmark.cc @@ -375,6 +375,76 @@ void RegisterZip() { // NOLINTEND(*-magic-numbers) +// Control the early-success and missing-name paths separately from last-member replay. +// Fresh means a new member wrapper, not cold operating-system caches: every iteration +// opens a new archive cursor, while the immutable memory fixture stays resident. +void MeasureMemberOpenControl(benchmark::State& state, bool missing, bool fresh) { + constexpr std::size_t kPayload = 128; + auto bytes = MakeTar(static_cast(state.range(0)), kPayload); + if (!bytes.ok()) { + state.SkipWithError(bytes.status().ToString()); + return; + } + const auto source = vfs::MemoryReadSource(std::move(*bytes)); + const std::string_view name = missing ? "absent" : "file-0"; + const std::string request = state.range(1) == 0 ? std::string(name) : absl::StrCat("././", name, "///"); + const auto member = MemberReadSource(source, request); + const std::string expected(kPayload, 'x'); + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores): benchmark loop controls measured iterations. + for (const auto unused : state) { + auto result = fresh ? vfs::ReadSourceBytes(*MemberReadSource(source, request), kPayload) + : vfs::ReadSourceBytes(*member, kPayload); + if (missing ? result.status().code() != absl::StatusCode::kNotFound : (!result.ok() || *result != expected)) { + state.SkipWithError("first/missing member control differs from its fixture"); + return; + } + benchmark::DoNotOptimize(result); + } + state.SetItemsProcessed(state.iterations()); +} + +void RegisterMemberOpenControls() { + struct ControlCase { + std::string_view name; + bool missing; + bool fresh; + }; + + constexpr auto kCases = std::to_array({ + { + .name = "first/reopen", + .missing = false, + .fresh = false, + }, + { + .name = "first/fresh-wrapper", + .missing = false, + .fresh = true, + }, + { + .name = "missing/reopen", + .missing = true, + .fresh = false, + }, + { + .name = "missing/fresh-wrapper", + .missing = true, + .fresh = true, + }, + }); + constexpr auto kCounts = std::to_array({10, 10'000}); + for (const auto& control : kCases) { + const std::string name = absl::StrCat("archive-open-control/", control.name); + // XFF_ABI_POINTER: Google Benchmark returns a borrowed mutable registration object. + auto* registration = benchmark::RegisterBenchmark(name, MeasureMemberOpenControl, control.missing, control.fresh); + for (const auto count : kCounts) { + registration->Args({count, 0}); + registration->Args({count, 1}); + } + registration->Unit(benchmark::kMicrosecond)->UseRealTime()->MinTime(0.02)->MinWarmUpTime(0.01)->Repetitions(9); + } +} + void RegisterAll() { constexpr auto kSources = std::to_array({ {.name = "legacy", .kind = SourceKind::kLegacy}, @@ -399,6 +469,7 @@ void RegisterAll() { } RegisterLarge(kSources); RegisterZip(); + RegisterMemberOpenControls(); } } // namespace