diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 45e7881a628..08ac5b0197f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -736,7 +736,7 @@ jobs: include: - os: ubuntu-latest platform: linux - timeout: 30 + timeout: 60 - os: macos-latest platform: macos timeout: 60 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d75d3689d4e..6079492fa9f 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)\.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-20261103)\.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 cd7880f0a4d..6a0a76566c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 0.11.0 +- Reduce cached-digest copies and plain/escaped output allocation churn while preserving snapshot lifetimes and exact output bytes. + - Reuse a session-owned archive input buffer when the source supports caller-owned reads, preserving decoder fallbacks and read budgets. - Avoid repeated word-window string construction in text similarity checks while preserving exact byte-word scores. diff --git a/TODO.md b/TODO.md index 08419744062..eff6ba6e5cf 100644 --- a/TODO.md +++ b/TODO.md @@ -457,9 +457,17 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms equality shortcut on Linux/macOS. Preserve [rejected width-one paths and all controls](docs/benchmarks/byte-shingle-representations-20261102.md). - [ ] O21b: Reuse immutable-source/reference representations and exact candidate indexes only after resolving dynamic target/invalidation and retained-byte contracts; keep run-wide grouping semantics explicit. -- [ ] O22: Qualify snapshot-owned digest observers, invariant field-hash qualifiers and output - construction separately. Preserve lifetime/invalidation, direct fallback and formatting contracts. - Do not revive O05's rejected generic lookup change without new consumer-specific evidence. +- [x] O22a: Qualify snapshot-owned digest observers against owning consumption, preserving cache + insertion stability, invalidation, local-snapshot fallback, read errors and derived handoff. + Keep [the separate mechanisms and controls](docs/benchmarks/digest-and-record-storage-20261103.md). +- [ ] O22b: Qualify invariant field-hash binding without enlarging every unrelated template segment + or hiding typed state in strings. Preserve dynamic defaults and cold/non-hash controls. +- [x] O22c1: Qualify direct plain-record construction and bulk C-escaping on both hosts, retaining + tiny colored controls and shared display/tree consumers in the O22a report. +- [ ] O22c2: Qualify command-owned reusable record/row buffers and streaming machine formats. + Preserve synchronous view lifetimes, reentrant callbacks, coordinator ownership and ordered + publication. Do not add a buffer/destructor to every existing stateless renderer by default. + O22a/c1 do not complete this work or revive O05's rejected generic lookup change. - [ ] Qualify executor admission after the clock correction with separate cold/warm controls and 16/32/63/64-entry matching boundaries; check Linux and macOS workload regressions before diff --git a/bazelmod/clang_tidy_targets.bzl b/bazelmod/clang_tidy_targets.bzl index 855fcf18747..1fc429e5359 100644 --- a/bazelmod/clang_tidy_targets.bzl +++ b/bazelmod/clang_tidy_targets.bzl @@ -23,4 +23,5 @@ CLANG_TIDY_MANUAL_TARGETS = [ "//xff/matching/similarity:near_duplicate_benchmark", "//xff/matching/similarity:similarity_benchmark", "//xff/parser:parser_benchmark", + "//xff/presentation/render:output_benchmark", ] diff --git a/docs/benchmarks/digest-and-record-storage-20261103.json.gz b/docs/benchmarks/digest-and-record-storage-20261103.json.gz new file mode 100644 index 00000000000..b476f6ba1bd Binary files /dev/null and b/docs/benchmarks/digest-and-record-storage-20261103.json.gz differ diff --git a/docs/benchmarks/digest-and-record-storage-20261103.md b/docs/benchmarks/digest-and-record-storage-20261103.md new file mode 100644 index 00000000000..be569fb14af --- /dev/null +++ b/docs/benchmarks/digest-and-record-storage-20261103.md @@ -0,0 +1,128 @@ +# Borrowed digests and record storage (2026-11-03) + +## Decision + +Borrow already cached digest bytes for hash actions, equality checks and supplied-snapshot fields. Keep the owning API and the local-snapshot field fallback. Build plain records directly in their final storage; bulk-append printable runs when C-escaping instead of mutating the string for every ordinary byte. + +The digest cache layout and lookup policy do not change. Map-node storage remains stable across other insertions; invalidation, transfer, moving or destroying the snapshot ends the public borrow contract. Action output owns its final concatenation before calling the sink. Field transforms consume the view while the supplied snapshot is alive. No new cache, lock, worker, flag, dependency, exception path or host-I/O route is introduced. + +Tiny colored records have slower isolated cells even where absolute changes are only a few nanoseconds. Retain those controls. O22b invariant hash-field binding and O22c command-owned reusable output/row buffers remain open; this change does not complete them. + +## Measurement contracts + +Fixed parent `35f404af9e29c4d1e13db57bdec923b91d143b3f` (#1066). Four canonical variants separate the mechanism: `baseline`, `digest`, `record` (direct construction plus bulk C-escaping), and `both`. Common benchmark/test support is outside the runtime variants. Build manifests retain compiler bytes/version, source/binary hashes, effective release/ThinLTO settings and link actions. Later dependency changes require fresh qualification. + +Per host: 40 native cases over 10/1,000/10,000 files, longer paths and larger content, one/three requested workers, listing, hash actions, repeated hashes, expected digest templates and repeated/transformed fields; three sessions, nine rotating rounds and fastest seven retained. Independent Python hashlib values and complete stdout/zero exits are checked. macOS uses warm APFS; Linux uses tmpfs with CPU affinity. Requested workers do not imply newly parallel hash actions. + +The in-memory matrix has 82 cases x three sessions x four variants. Final memory runs batch at most eight cases with the same benchmark family and CPU allowance per process, rotate participant/block order, and retain each case's own warmup and nine timed repetitions. This amortizes an untimed Google Benchmark 1.9.5 CPU metadata fallback: missing macOS `hw.cpufrequency` otherwise triggers a one-second busy-loop estimate per fresh process. The reported 24 MHz is the ARM counter, not core frequency. A three-case pilot verifies exact selection/repetition counts. The complete separate Linux case-isolated matrix is retained. + +Confirmation adds an identical baseline participant: eight native cases x five sessions and 14 memory cases x three sessions. Duplicate-baseline movement is shown without subtracting it. Timing does not overlap builds, lint, sanitizers or allocation probes on the same host. + +## Native commands + +Median paired session change; negative means less elapsed time. + +| Fixture / action | CPUs | Linux | macOS | +| :------------------------- | ---: | -----: | -----: | +| flat10 / listing | 1 | +0.44% | +0.62% | +| flat10000 / listing | 1 | -0.62% | +1.73% | +| flat10000 / listing | 3 | +1.93% | -0.06% | +| long1000 / listing | 1 | +0.18% | +0.17% | +| flat10000 / hash-repeat | 1 | +0.42% | +0.11% | +| flat10000 / fields | 3 | +1.22% | -1.37% | +| flat1000 / hasheq-template | 1 | -0.97% | +0.69% | +| large1000 / hash-repeat | 1 | +0.67% | +0.02% | + +## Confirmation controls + +| Fixture / action | CPUs | Linux candidate | Linux repeat | macOS candidate | macOS repeat | +| :------------------------- | ---: | --------------: | -----------: | --------------: | -----------: | +| flat10000 / hash-repeat | 1 | +0.26% | -0.11% | -0.08% | +0.35% | +| flat10 / listing | 1 | +3.30% | +0.71% | +2.61% | +0.39% | +| flat10000 / fields | 3 | +0.05% | +0.89% | -0.02% | +0.44% | +| flat1000 / hasheq-template | 1 | +1.32% | +0.32% | +0.68% | +0.32% | +| long1000 / listing | 1 | +1.39% | -0.59% | +1.11% | +0.99% | +| flat10000 / listing | 3 | +3.22% | +0.77% | +0.12% | +0.05% | +| large1000 / hash-repeat | 1 | +1.29% | +0.19% | +0.23% | -0.14% | +| flat10000 / listing | 1 | +3.55% | +0.87% | +1.06% | +0.58% | + +## Largest native slowdowns and controls + +Largest paired changes across all native cases, including near-neutral controls. Absolute times and paired percentage changes are separate medians. + +| Host | Fixture / action | CPUs | Baseline ms | Selected ms | Change | +| :---- | :-------------------------- | ---: | ----------: | ----------: | -----: | +| linux | flat10 / fields-transform | 1 | 0.872 | 0.901 | +3.01% | +| linux | flat10 / hash-sha | 1 | 0.880 | 0.904 | +2.66% | +| linux | flat1000 / fields | 3 | 3.418 | 3.441 | +2.35% | +| linux | flat1000 / hash-sha | 3 | 2.631 | 2.690 | +2.15% | +| macos | flat10000 / listing | 1 | 12.695 | 12.853 | +1.73% | +| macos | flat1000 / fields-transform | 3 | 19.477 | 19.598 | +1.07% | +| macos | flat10 / hasheq-template | 3 | 4.622 | 4.670 | +1.05% | +| macos | flat1000 / hasheq-template | 1 | 19.683 | 19.630 | +0.69% | + +## Isolated and whole-engine attribution + +Every cell is a paired three-session median; baseline values use the unit named in the case. Whole-engine iterations include preparation, traversal, metadata/content, sinks and teardown; parsing/fixture construction are outside timing. Isolated field cases distinguish freshly invalidated (`0`) from cached (`1`) snapshots. + +| Case | Host | Baseline | Unit | Digest only | Record only | Both | +| :------------------------------------------- | :---- | --------: | :--- | ----------: | ----------: | ------: | +| output/record/1/0/1 | linux | 13.61 | ns | -0.12% | +39.75% | +42.07% | +| output/record/1/0/1 | macos | 10.06 | ns | -3.70% | +94.97% | +96.27% | +| output/record/160/0/0 | linux | 20.87 | ns | +0.32% | -60.85% | -59.63% | +| output/record/160/0/0 | macos | 35.80 | ns | +1.06% | -35.28% | -34.43% | +| output/record/160/1/1 | linux | 293.62 | ns | +9.07% | -73.26% | -71.66% | +| output/record/160/1/1 | macos | 527.36 | ns | -0.07% | -62.99% | -58.14% | +| output/hash-fields/16/0 | linux | 942.96 | ns | -22.35% | +0.42% | -22.51% | +| output/hash-fields/16/0 | macos | 1,771.35 | ns | -15.58% | -0.66% | -17.75% | +| output/hash-fields/16/1 | linux | 703.88 | ns | -30.15% | -2.99% | -31.61% | +| output/hash-fields/16/1 | macos | 1,313.09 | ns | -21.25% | +0.84% | -24.89% | +| engine/production/hash-fields-repeat/10000/1 | linux | 9,210.13 | us | -8.02% | -3.40% | -7.84% | +| engine/production/hash-fields-repeat/10000/1 | macos | 17,650.84 | us | -8.46% | -1.16% | -7.43% | +| engine/production/hash-repeat/10000/1 | linux | 10,897.84 | us | -3.88% | -0.58% | -3.99% | +| engine/production/hash-repeat/10000/1 | macos | 18,096.61 | us | -3.82% | -0.42% | -3.85% | + +## Slower isolated controls + +Largest output-kernel regressions, retaining absolute time alongside percentages. These are isolated records/fields, not whole-command slowdowns. + +| Host | Case | Baseline | Selected | Unit | Change | +| :---- | :--------------------- | -------: | -------: | :--- | ------: | +| linux | output/record/1/0/1 | 13.61 | 19.30 | ns | +42.07% | +| linux | output/record/1/1/1 | 23.67 | 31.69 | ns | +34.24% | +| linux | output/record/1/1/0 | 20.01 | 20.71 | ns | +3.41% | +| linux | output/hash-fields/1/0 | 287.92 | 276.60 | ns | -4.17% | +| macos | output/record/1/0/1 | 10.06 | 19.75 | ns | +96.27% | +| macos | output/record/1/1/1 | 23.68 | 40.21 | ns | +69.83% | +| macos | output/record/15/0/1 | 25.97 | 36.49 | ns | +36.96% | +| macos | output/record/1/1/0 | 21.30 | 27.75 | ns | +28.05% | + +## Allocation diagnostics + +Separate warmed calling-thread ASan counters count allocation requests/bytes only. Fixture setup and reporting are excluded; cold fields include invalidation. These are not release timings, retained memory or peak RSS. + +| Case | Host | Baseline requests | Selected requests | Baseline bytes | Selected bytes | +| :---------------------- | :---- | ----------------: | ----------------: | -------------: | -------------: | +| output/record/160/0/0 | linux | 2 | 1 | 352 | 176 | +| output/record/160/0/0 | macos | 2 | 1 | 336 | 168 | +| output/record/160/1/1 | linux | 4 | 2 | 528 | 528 | +| output/record/160/1/1 | macos | 4 | 2 | 520 | 528 | +| output/hash-fields/16/1 | linux | 21 | 5 | 3,760 | 2,480 | +| output/hash-fields/16/1 | macos | 21 | 5 | 3,384 | 2,232 | + +## Correctness and footprint + +Both hosts pass all 325 extension-enabled release suites and eight focused ASan suites. Linux additionally passes eight focused MSan and eight TSan suites, with FUSE required for release testing. Exact selected-source guards precede final validation. Lifetime, insertion stability, owning fallback, invalidation, cached errors, derived handoff, empty/binary bytes, escape boundaries, adjacent controls and tree/display consumers are exercised. Real scoped lint and failed-file refinements are retained; normal hooks/current-head CI remain required for PR readiness. + +- linux: unstripped full CLI 18,121,664 -> 18,120,272 bytes (-1,392). +- macos: unstripped full CLI 8,140,216 -> 8,140,216 bytes (+0). + +## Rejected prototype and remaining work + +The first combined prototype reduced raw-record allocations but regressed 160-byte escaped Linux records by roughly 9-10% in confirmation. It is not selected. Its full Linux matrix, confirmation, sanitizer/allocation evidence, complete Mac native matrix and explicitly partial Mac memory population are retained. Remaining Mac prototype work was intentionally stopped after rejection; no full Mac validation is claimed for it. The revised candidate separately qualifies bulk printable-run appends and all affected escape consumers. + +Explicit hash-field binding and command-owned reusable record/row buffers remain separate O22 work. Source/archive replay, coordinated workers and bounded pipeline overlap remain open in the main inventory. + +## Lossless evidence + +[digest-and-record-storage-20261103.json.gz](digest-and-record-storage-20261103.json.gz): 3,646,571 bytes; SHA-256 `5841e5615cbc286b7fb77820f2049e99d7523b74e0c9ebd38e6694f195fa7faa`. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index e38b7fab15e..3d3bb59b014 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -3509,3 +3509,47 @@ This covers O21a's per-comparison representations only. O21b reference reuse and remain open: dynamic peer templates, content mutation/invalidation, immutable-source identity, retained-byte bounds and grouping semantics require their own qualification. No source-independent cross-file cache or approximate relationship is introduced. + +### O22a/O22c1: digest observers and record construction + +Hash actions, equality checks and fields previously copied strings that the entry snapshot already +owned. `DigestView` exposes those immutable bytes without changing the map or its lookup policy. +Other digest insertions preserve storage; invalidation, transfer, moving and destruction end the +borrow contract. Actions own their final emitted text before calling the sink. Supplied-snapshot +fields consume views while their owner lives; an extraction with a local snapshot retains its +owning digest fallback. The owning API remains available. This does not add a cross-file cache, +shared state, locks or worker admission. + +Plain records previously constructed a path body and a second final string. The selected path +builds the final record directly. C-escaping appends printable runs in bulk instead of mutating the +string for every byte. Exact control-byte escapes, high bytes, color/reset/newline behavior and +shared display/tree consumers remain unchanged. Escape expansion can still grow storage; do not +claim one allocation for every output shape. + +[The qualification report](benchmarks/digest-and-record-storage-20261103.md) separates digest-only, +record-only and combined variants, native and in-memory work, duplicate-baseline controls, +allocation diagnostics and footprint. The first combined prototype regressed escaped records on +Linux and is not selected. Its completed populations, including explicitly partial Mac memory +measurements stopped after rejection, remain separate from the revised bulk-append candidate. +Tiny colored records retain slower isolated cells; absolute nanoseconds and percentages accompany +the favorable larger-record and repeated-field measurements. These are not universal speedups. + +The qualification driver also exposed an untimed Google Benchmark 1.9.5 startup cost on this Mac: +missing `hw.cpufrequency` makes `GetCPUCyclesPerSecond` run a one-second busy estimate in every +fresh process. The reported 24 MHz describes the ARM counter, not CPU core frequency. This is +measurement-tool metadata overhead, not XFF startup or measured iteration time. Final memory +qualification batches at most eight same-family/same-CPU cases per process, retaining each case's +own warmup, nine repetitions and fastest-seven selection with rotating participant/block order. +The Linux case-isolated matrix remains separate; compare within a recorded protocol rather than +silently treating absolute measurements from different protocols as interchangeable. + +O22b still needs typed field-hash binding. Compact IDs/default-use bits may fit existing segment +padding, but actual layout, renderer-call overhead, changing context defaults and cold/non-hash +controls must be qualified before using that space. Do not encode binding state in arbitrary +strings or add a view-rich allocation to every segment. + +O22c2 still needs command-owned reusable output buffers and streaming rows. Keep owning/stateless +APIs intact; a private stateful record writer could reuse capacity on the coordinator and lend a +view only during the synchronous sink call. Audit aliasing, reentrant callbacks, cancellation and +worker/result lifetimes. Worker-produced output must stay owned or have a separately proven batch +lifetime. This is separate from O18 byte backpressure and ordered pipeline publication. diff --git a/release-site.json b/release-site.json index c1856983779..42af27f9334 100644 --- a/release-site.json +++ b/release-site.json @@ -77,7 +77,8 @@ "docs/benchmarks/prepared-hash-operands-20261102.md": "docs/benchmarks/prepared-hash-operands-20261102.html", "docs/benchmarks/aggregate-comparison-totals-20261102.md": "docs/benchmarks/aggregate-comparison-totals-20261102.html", "docs/benchmarks/byte-shingle-representations-20261102.md": "docs/benchmarks/byte-shingle-representations-20261102.html", - "docs/benchmarks/archive-input-buffers-20261102.md": "docs/benchmarks/archive-input-buffers-20261102.html" + "docs/benchmarks/archive-input-buffers-20261102.md": "docs/benchmarks/archive-input-buffers-20261102.html", + "docs/benchmarks/digest-and-record-storage-20261103.md": "docs/benchmarks/digest-and-record-storage-20261103.html" }, "generated_html": { "XFF.md": { diff --git a/xff/content/snapshot.cc b/xff/content/snapshot.cc index 233095f0b13..a244d9ec9f6 100644 --- a/xff/content/snapshot.cc +++ b/xff/content/snapshot.cc @@ -36,6 +36,14 @@ std::optional Snapshot::Digest( return DigestValue(fs, path, spec); } +std::optional Snapshot::DigestView( + const vfs::FileSystem& fs, + std::string_view path, + const hash::AlgoEncoding& spec) const { + const auto& value = DigestValue(fs, path, spec); + return value ? std::optional(*value) : std::nullopt; +} + const std::optional& Snapshot::DigestValue( const vfs::FileSystem& fs, std::string_view path, diff --git a/xff/content/snapshot.h b/xff/content/snapshot.h index 6f52456f925..b9097867b2c 100644 --- a/xff/content/snapshot.h +++ b/xff/content/snapshot.h @@ -42,6 +42,13 @@ class Snapshot final { absl::StatusOr Read(const vfs::FileSystem& fs, std::string_view path) const; std::optional Digest(const vfs::FileSystem& fs, std::string_view path, const hash::AlgoEncoding& spec) const; + // Borrow a cached digest without copying its string. Inserting other digests preserves the + // view; invalidating, transferring, moving or destroying this snapshot ends the borrow. + // Read failures and unsupported algorithms remain absent, as with the owning Digest form. + std::optional DigestView( + const vfs::FileSystem& fs, + std::string_view path, + const hash::AlgoEncoding& spec) const; std::optional Lines(const vfs::FileSystem& fs, std::string_view path) const; void Invalidate(); diff --git a/xff/content/snapshot_test.cc b/xff/content/snapshot_test.cc index 0dc423fb09f..93b25e30afa 100644 --- a/xff/content/snapshot_test.cc +++ b/xff/content/snapshot_test.cc @@ -168,5 +168,79 @@ TEST_F(SnapshotTest, KeepsBinaryClassificationAndMovesOwnedState) { EXPECT_THAT(moved.Lines(fs, "file"), Eq(std::nullopt)); EXPECT_THAT(fs.reads, Eq(1)); } + +TEST_F(SnapshotTest, DigestViewsBorrowStableCacheStorageAcrossOtherDigestInsertions) { + const auto first = snapshot.DigestView(fs, "file", {.algo = "sha256"}); + const auto expected = hash::HashData("sha256", fs.bytes).value_or("unsupported"); + ASSERT_THAT(first, Optional(EqualsText(expected))); + EXPECT_THAT( + snapshot.DigestView(fs, "file", {.algo = "md5"}), + Optional(EqualsText(hash::HashData("md5", fs.bytes).value_or("unsupported")))); + EXPECT_THAT( + snapshot.DigestView(fs, "file", {.algo = "sha256", .encoding = hash::Encoding::kBase64}), + Optional(EqualsText(hash::HashData("sha256", fs.bytes, hash::Encoding::kBase64).value_or("unsupported")))); + const auto repeated = snapshot.DigestView(fs, "file", {.algo = "sha256"}); + EXPECT_THAT(first, Optional(EqualsText(expected))); + EXPECT_THAT(repeated, Optional(EqualsText(expected))); + EXPECT_THAT(first.value_or("").begin(), Eq(repeated.value_or("").begin())); + EXPECT_THAT(fs.reads, Eq(1)); +} + +TEST_F(SnapshotTest, OwnedDigestsSurviveInvalidationWhileFreshViewsObserveReplacementBytes) { + const auto original = hash::HashData("sha256", fs.bytes).value_or("unsupported"); + const auto owned = snapshot.Digest(fs, "file", {.algo = "sha256"}); + EXPECT_THAT(snapshot.DigestView(fs, "file", {.algo = "sha256"}), Optional(EqualsText(original))); + fs.bytes = "replacement"; + snapshot.Invalidate(); + EXPECT_THAT(owned, Optional(EqualsText(original))); + EXPECT_THAT( + snapshot.DigestView(fs, "file", {.algo = "sha256"}), + Optional(EqualsText(hash::HashData("sha256", fs.bytes).value_or("unsupported")))); + EXPECT_THAT(fs.reads, Eq(2)); +} + +TEST_F(SnapshotTest, DigestViewsPreserveBinaryEmptyAndFailedReadContracts) { + for (const auto& bytes : {std::string(), std::string("a\0b", 3)}) { + fs.bytes = bytes; + snapshot.Invalidate(); + for (const auto encoding : {hash::Encoding::kHex, hash::Encoding::kBase64}) { + for (const std::string_view algorithm : {"md5", "sha256"}) { + const hash::AlgoEncoding spec{.algo = algorithm, .encoding = encoding}; + EXPECT_THAT( + snapshot.DigestView(fs, "file", spec), + Optional(EqualsText(hash::HashData(algorithm, bytes, encoding).value_or("unsupported")))); + EXPECT_THAT( + snapshot.Digest(fs, "file", spec), + Optional(EqualsText(snapshot.DigestView(fs, "file", spec).value_or("")))); + } + } + } + EXPECT_THAT(snapshot.DigestView(fs, "file", {.algo = "unsupported"}), Eq(std::nullopt)); + fs.fail = true; + snapshot.Invalidate(); + EXPECT_THAT(snapshot.DigestView(fs, "file", {.algo = "sha256"}), Eq(std::nullopt)); + EXPECT_THAT(snapshot.Digest(fs, "file", {.algo = "sha256"}), Eq(std::nullopt)); + fs.fail = false; + EXPECT_THAT(snapshot.DigestView(fs, "file", {.algo = "sha256"}), Eq(std::nullopt)); + snapshot.Invalidate(); + EXPECT_THAT( + snapshot.DigestView(fs, "file", {.algo = "sha256"}), + Optional(EqualsText(hash::HashData("sha256", fs.bytes).value_or("unsupported")))); +} + +TEST_F(SnapshotTest, TransferredDerivedDigestsOfferViewsWithoutReadingChangedEntryContent) { + const auto expected = hash::HashData("sha256", fs.bytes).value_or("unsupported"); + EXPECT_THAT(snapshot.DigestView(fs, "file", {.algo = "sha256"}), Optional(EqualsText(expected))); + auto derived = std::move(snapshot).TakeDerived({.digests = {{.algorithm = "sha256"}}}, fs, "file"); + fs.bytes = "replacement"; + EXPECT_THAT(derived.DigestView(fs, "file", {.algo = "sha256"}), Optional(EqualsText(expected))); + EXPECT_THAT(derived.Loaded(), IsFalse()); + EXPECT_THAT(fs.reads, Eq(1)); + derived.Invalidate(); + EXPECT_THAT( + derived.DigestView(fs, "file", {.algo = "sha256"}), + Optional(EqualsText(hash::HashData("sha256", fs.bytes).value_or("unsupported")))); + EXPECT_THAT(fs.reads, Eq(2)); +} } // namespace } // namespace xff::content diff --git a/xff/engine/evaluate.cc b/xff/engine/evaluate.cc index 780074eeaef..59be8a92943 100644 --- a/xff/engine/evaluate.cc +++ b/xff/engine/evaluate.cc @@ -1662,8 +1662,8 @@ std::optional ResolveHashRequest( // The entry's digest, read through the filesystem the entry came FROM. That indirection is what // lets -hash / -hasheq work on an archive member: hashing by path would look for `a.tar!x` on the // real filesystem and find nothing. Both routes read the whole entry anyway. -std::optional DigestOfEntry(const EvalContext& ctx, const hash::AlgoEncoding& spec) { - return ctx.content.Digest(ctx.fs, ctx.visit.path, spec); +std::optional DigestOfEntry(const EvalContext& ctx, const hash::AlgoEncoding& spec) { + return ctx.content.DigestView(ctx.fs, ctx.visit.path, spec); } // xff -hash[:ALGO[/ENCODING]]: an ACTION that prints the entry's digest and path as @@ -1680,7 +1680,7 @@ bool EvalHashWithOperand( if (!spec.has_value()) { return true; } - const std::optional digest = DigestOfEntry(ctx, *spec); + const std::optional digest = DigestOfEntry(ctx, *spec); if (digest.has_value()) { ctx.emit(absl::StrCat(*digest, " ", ctx.visit.path, "\n")); } @@ -1722,7 +1722,7 @@ bool EvalHasheqWithOperand( if (!spec.has_value()) { return verdict(false); // defensively no-op; ValidateHashArgs rejects a bad spec before the walk } - const std::optional digest = DigestOfEntry(ctx, *spec); + const std::optional digest = DigestOfEntry(ctx, *spec); if (!digest.has_value()) { return verdict(false); // unreadable / non-regular file -> mismatch (selected by `! -hasheq`) } diff --git a/xff/engine/expression_run_benchmark.cc b/xff/engine/expression_run_benchmark.cc index a66219a1f85..b71382baae3 100644 --- a/xff/engine/expression_run_benchmark.cc +++ b/xff/engine/expression_run_benchmark.cc @@ -191,6 +191,18 @@ int main(int argc, char** argv) { .name = "hash-default", .arguments = {"-type", "f", "-hash"}, }, + { + .name = "hash-repeat", + .arguments = {"-type", "f", "-hash", "-hash", "-hash"}, + }, + { + .name = "hash-fields-repeat", + .arguments = {"-type", "f", "--template={hash}|{hash}|{hash:md5/base64}|{hash:sha256/base64}"}, + }, + { + .name = "hash-fields-transform", + .arguments = {"-type", "f", "--template={hash:s/^/SUM-/}|{hash}|{hash:md5/base64}"}, + }, { .name = "hasheq-explicit", .arguments = {"-type", "f", "-hasheq:md5/base64", "V2pFZbcPWkwaCSXKvbWHpg=="}, diff --git a/xff/presentation/fields/BUILD.bazel b/xff/presentation/fields/BUILD.bazel index 8b540a2c938..d48f2f5c012 100644 --- a/xff/presentation/fields/BUILD.bazel +++ b/xff/presentation/fields/BUILD.bazel @@ -60,6 +60,25 @@ cc_test( ], ) +cc_test( + name = "hash_snapshot_test", + size = "small", + srcs = ["hash_snapshot_test.cc"], + deps = [ + ":fields_cc", + "//xff/content:snapshot_cc", + "//xff/hash:hash_cc", + "//xff/vfs:vfs_cc", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@abseil-cpp//absl/time", + "@googletest//:gtest", + "@googletest//:gtest_main", + "@mboworks_mbo//mbo/testing:matchers_cc", + "@mboworks_mbo//mbo/testing:status_cc", + ], +) + cc_test( name = "lexical_fields_test", size = "small", diff --git a/xff/presentation/fields/fields.cc b/xff/presentation/fields/fields.cc index 482e0d8c023..7e8c94307ab 100644 --- a/xff/presentation/fields/fields.cc +++ b/xff/presentation/fields/fields.cc @@ -314,8 +314,9 @@ StringOrView HashField(std::string_view, std::string_view qualifier, const Rende return ""; // unknown algorithm or encoding -> empty, like an unknown field } if (ctx.content) { - return ctx.content->Digest(ContentFileSystem(ctx), ctx.path, *spec).value_or(""); + return ctx.content->DigestView(ContentFileSystem(ctx), ctx.path, *spec).value_or(""); } + // This local snapshot dies on return, so the fallback must retain an owned digest. const content::Snapshot content; return content.Digest(ContentFileSystem(ctx), ctx.path, *spec).value_or(""); } diff --git a/xff/presentation/fields/hash_snapshot_test.cc b/xff/presentation/fields/hash_snapshot_test.cc new file mode 100644 index 00000000000..d31c1b6737b --- /dev/null +++ b/xff/presentation/fields/hash_snapshot_test.cc @@ -0,0 +1,127 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/time/time.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "mbo/testing/matchers.h" +#include "mbo/testing/status.h" +#include "xff/content/snapshot.h" +#include "xff/hash/hash.h" +#include "xff/presentation/fields/fields.h" +#include "xff/vfs/filesystem.h" + +namespace xff::fields { +namespace { + +using ::mbo::testing::EqualsText; +using ::mbo::testing::IsOk; +using ::testing::ElementsAre; +using ::testing::Eq; +using ::testing::Optional; + +struct HashFs final : vfs::FileSystem { + std::string bytes = "alpha\nbeta\n"; + bool fail = false; + mutable std::size_t reads = 0; + + absl::StatusOr ReadContent(std::string_view) const override { + ++reads; + return fail ? absl::StatusOr(absl::PermissionDeniedError("unreadable")) : bytes; + } + + absl::StatusOr> ReadDir(std::string_view) const override { + return absl::UnimplementedError("unused"); + } + + absl::StatusOr Stat(std::string_view, bool) const override { + return absl::UnimplementedError("unused"); + } + + absl::StatusOr IsCaseSensitive(std::string_view) const override { return true; } + + bool Access(std::string_view, vfs::AccessMode) const override { return false; } + + absl::StatusOr ReadLink(std::string_view) const override { return absl::UnimplementedError("unused"); } + + absl::StatusOr FsType(std::string_view) const override { return absl::UnimplementedError("unused"); } + + absl::Status Remove(std::string_view) const override { return absl::PermissionDeniedError("read-only"); } +}; + +struct HashSnapshotTest : ::testing::Test { + HashFs fs; + content::Snapshot snapshot; + const vfs::Metadata metadata; + + RenderContext Context(bool supplied) const { + RenderContext result{ + .path = "virtual/file", + .metadata = metadata, + .fs = fs, + .tz = absl::UTCTimeZone(), + }; + if (supplied) { + result.content.set_ref(snapshot); + } + return result; + } +}; + +TEST_F(HashSnapshotTest, RepeatedHashFieldsUseOneReadWithBothLocalAndSuppliedSnapshots) { + const auto compiled = Template::Compile("{hash}|{hash:md5/base64}|{hash}|{hash:sha256/base64}"); + const std::string sha = hash::HashData("sha256", fs.bytes).value_or("unsupported"); + const std::string expected = sha + "|" + hash::HashData("md5", fs.bytes, hash::Encoding::kBase64).value_or("") + "|" + + sha + "|" + hash::HashData("sha256", fs.bytes, hash::Encoding::kBase64).value_or(""); + EXPECT_THAT(compiled.Render(Context(false)), EqualsText(expected)); + EXPECT_THAT(fs.reads, Eq(1)); + EXPECT_THAT(compiled.Render(Context(true)), EqualsText(expected)); + EXPECT_THAT(compiled.Render(Context(true)), EqualsText(expected)); + EXPECT_THAT(fs.reads, Eq(2)); +} + +TEST_F(HashSnapshotTest, RenderedOutputOwnsItsDigestAfterTheEntrySnapshotIsInvalidated) { + const auto compiled = Template::Compile("{hash}|{hash}"); + const std::string original = hash::HashData("sha256", fs.bytes).value_or("unsupported"); + const std::string rendered = compiled.Render(Context(true)); + fs.bytes = "replacement"; + EXPECT_THAT(compiled.Render(Context(true)), EqualsText(original + "|" + original)); + snapshot.Invalidate(); + const std::string replacement = hash::HashData("sha256", fs.bytes).value_or("unsupported"); + EXPECT_THAT(compiled.Render(Context(true)), EqualsText(replacement + "|" + replacement)); + EXPECT_THAT(rendered, EqualsText(original + "|" + original)); + EXPECT_THAT(fs.reads, Eq(2)); +} + +TEST_F(HashSnapshotTest, ExtractionWithoutASuppliedSnapshotRetainsTheLocalDigestUntilConsumed) { + const auto compiled = Template::Compile(R"({hash:"m|^(.{8}).*|\1|"})"); + ASSERT_THAT(compiled.Validate(), IsOk()); + const std::string expected = hash::HashData("sha256", fs.bytes).value_or("unsupported").substr(0, 8); + for (const bool supplied : {false, true}) { + EXPECT_THAT(compiled.AsExtraction(Context(supplied)), Optional(ElementsAre(EqualsText(expected)))); + } + EXPECT_THAT(fs.reads, Eq(2)); +} + +TEST_F(HashSnapshotTest, UnknownAlgorithmsAndReadFailuresStayEmptyUntilInvalidation) { + const auto compiled = Template::Compile("[{hash:unknown}][{hash}][{hash}]"); + fs.fail = true; + EXPECT_THAT(compiled.Render(Context(true)), EqualsText("[][][]")); + fs.fail = false; + EXPECT_THAT(compiled.Render(Context(true)), EqualsText("[][][]")); + EXPECT_THAT(fs.reads, Eq(1)); + snapshot.Invalidate(); + const std::string expected = hash::HashData("sha256", fs.bytes).value_or("unsupported"); + EXPECT_THAT(compiled.Render(Context(true)), EqualsText("[][" + expected + "][" + expected + "]")); + EXPECT_THAT(fs.reads, Eq(2)); +} + +} // namespace +} // namespace xff::fields diff --git a/xff/presentation/render/BUILD.bazel b/xff/presentation/render/BUILD.bazel index a66a3bf462e..a28d0f6eb23 100644 --- a/xff/presentation/render/BUILD.bazel +++ b/xff/presentation/render/BUILD.bazel @@ -13,10 +13,31 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("//xff:cc.bzl", "cc_library", "cc_test") +load("//xff:cc.bzl", "cc_binary", "cc_library", "cc_test") package(default_visibility = ["//visibility:private"]) +cc_binary( + name = "output_benchmark", + testonly = True, + srcs = ["output_benchmark.cc"], + tags = [ + "clang-tidy", + "manual", + ], + deps = [ + ":render_cc", + "//xff/content:snapshot_cc", + "//xff/hash:hash_cc", + "//xff/presentation/fields:fields_cc", + "//xff/vfs:vfs_cc", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@abseil-cpp//absl/time", + "@com_github_google_benchmark//:benchmark", + ], +) + # Output record formatting for matched paths: plain (\n), NUL (\0), JSONL. cc_library( name = "render_cc", diff --git a/xff/presentation/render/output_benchmark.cc b/xff/presentation/render/output_benchmark.cc new file mode 100644 index 00000000000..6c9dcbb73a1 --- /dev/null +++ b/xff/presentation/render/output_benchmark.cc @@ -0,0 +1,124 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +// Isolate record construction and cached digest consumption from host filesystem costs. +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/time/time.h" +#include "benchmark/benchmark.h" +#include "xff/content/snapshot.h" +#include "xff/hash/hash.h" +#include "xff/presentation/fields/fields.h" +#include "xff/presentation/render/render.h" +#include "xff/vfs/filesystem.h" + +namespace xff::render { +namespace { + +struct OutputFs final : vfs::FileSystem { + absl::StatusOr ReadContent(std::string_view) const override { return "alpha\nbeta\n"; } + + absl::StatusOr> ReadDir(std::string_view) const override { + return absl::UnimplementedError("unused"); + } + + absl::StatusOr Stat(std::string_view, bool) const override { + return absl::UnimplementedError("unused"); + } + + absl::StatusOr IsCaseSensitive(std::string_view) const override { return true; } + + bool Access(std::string_view, vfs::AccessMode) const override { return false; } + + absl::StatusOr ReadLink(std::string_view) const override { return absl::UnimplementedError("unused"); } + + absl::StatusOr FsType(std::string_view) const override { return absl::UnimplementedError("unused"); } + + absl::Status Remove(std::string_view) const override { return absl::PermissionDeniedError("read-only"); } +}; + +void Record(benchmark::State& state) { + const std::string prefix(static_cast(state.range(0)), 'x'); + const std::string path = prefix + "\n\\\t"; + const bool escape = state.range(1) != 0; + const std::string_view color = state.range(2) != 0 ? "1;34" : ""; + const Renderer renderer(Format::kPlain, escape ? PathEncoding::kEscape : PathEncoding::kRaw); + const std::string body = escape ? prefix + R"(\n\\\t)" : path; + const std::string expected = color.empty() ? body + "\n" : "\x1b[1;34m" + body + "\x1b[0m\n"; + if (renderer.Record(path, color) != expected) { + state.SkipWithError("record bytes differ from independently constructed expectation"); + return; + } + for (auto iteration : state) { + benchmark::DoNotOptimize(iteration); + auto record = renderer.Record(path, color); + benchmark::DoNotOptimize(record); + } +} + +void HashFields(benchmark::State& state) { + const auto count = static_cast(state.range(0)); + const bool warm = state.range(1) != 0; + const OutputFs fs; + const vfs::Metadata metadata; + content::Snapshot snapshot; + const fields::RenderContext context{ + .path = "virtual/file", + .metadata = metadata, + .fs = fs, + .content = snapshot, + .tz = absl::UTCTimeZone(), + }; + std::string format; + std::string expected; + const std::string hash = hash::HashData("sha256", "alpha\nbeta\n").value_or("unsupported"); + for (std::size_t index = 0; index < count; ++index) { + format.append("{hash}|"); + expected.append(hash).push_back('|'); + } + const auto compiled = fields::Template::Compile(format); + if (!compiled.Validate().ok() || compiled.Render(context) != expected) { + state.SkipWithError("digest fields differ from known digest values"); + return; + } + for (auto iteration : state) { + benchmark::DoNotOptimize(iteration); + if (!warm) { + snapshot.Invalidate(); + } + auto rendered = compiled.Render(context); + benchmark::DoNotOptimize(rendered); + } +} + +} // namespace +} // namespace xff::render + +// XFF_ABI_POINTER: Google Benchmark's process argument interface. +int main(int argc, char** argv) { + benchmark::Initialize(&argc, argv); + if (benchmark::ReportUnrecognizedArguments(argc, argv)) { + return 1; + } + benchmark::RegisterBenchmark("output/record", xff::render::Record) + ->ArgsProduct({{1, 15, 22, 23, 64, 160, 4'096}, {0, 1}, {0, 1}}) + ->UseRealTime() + ->Unit(benchmark::kNanosecond) + ->Repetitions(9) + ->MinTime(0.02) + ->MinWarmUpTime(0.01); + benchmark::RegisterBenchmark("output/hash-fields", xff::render::HashFields) + ->ArgsProduct({{1, 4, 16}, {0, 1}}) + ->UseRealTime() + ->Unit(benchmark::kNanosecond) + ->Repetitions(9) + ->MinTime(0.02) + ->MinWarmUpTime(0.01); + benchmark::RunSpecifiedBenchmarks(); + benchmark::Shutdown(); +} diff --git a/xff/presentation/render/render.cc b/xff/presentation/render/render.cc index e470774c6e3..639a9ad8759 100644 --- a/xff/presentation/render/render.cc +++ b/xff/presentation/render/render.cc @@ -63,23 +63,27 @@ void AppendJsonEscaped(std::string_view path, std::string& out) { // ASCII and high (UTF-8) bytes pass through verbatim. void AppendCEscaped(std::string_view path, std::string& out) { static constexpr std::string_view kHex = "0123456789ABCDEF"; - for (const char ch : path) { + std::size_t first = 0; + for (std::size_t position = 0; position < path.size(); ++position) { + const char ch = path[position]; const auto byte = static_cast(ch); + if (byte >= 0x20 && byte != 0x7F && ch != '\\') { + continue; + } + out.append(path.substr(first, position - first)); switch (ch) { case '\\': out.append("\\\\"); break; case '\n': out.append("\\n"); break; case '\t': out.append("\\t"); break; case '\r': out.append("\\r"); break; default: - if (byte < 0x20 || byte == 0x7F) { - out.append("\\x"); - out.push_back(kHex[static_cast(byte) >> 4U]); - out.push_back(kHex[static_cast(byte) & 0x0FU]); - } else { - out.push_back(ch); - } + out.append("\\x"); + out.push_back(kHex[static_cast(byte) >> 4U]); + out.push_back(kHex[static_cast(byte) & 0x0FU]); } + first = position + 1; } + out.append(path.substr(first)); } // Appends `field` to `out` as one RFC-4180 CSV field: quoted only when it contains a @@ -173,16 +177,22 @@ std::string Renderer::Record(std::string_view path, std::string_view color) cons return record; } case Format::kPlain: { - std::string body; // the path bytes (raw or C-escaped), before color + newline + std::string record; + // SGR prefix/suffix plus newline need eight bytes in addition to the color parameter. + record.reserve(path.size() + (color.empty() ? 1 : color.size() + 8)); + if (!color.empty()) { + absl::StrAppend(&record, "\x1b[", color, "m"); + } if (encoding_ == PathEncoding::kEscape) { - AppendCEscaped(path, body); + AppendCEscaped(path, record); } else { - body = std::string(path); + record.append(path); } - if (color.empty()) { - return absl::StrCat(body, "\n"); + if (!color.empty()) { + record.append("\x1b[0m"); } - return absl::StrCat("\x1b[", color, "m", body, "\x1b[0m\n"); + record.push_back('\n'); + return record; } case Format::kTsv: { std::string record; diff --git a/xff/presentation/render/render_test.cc b/xff/presentation/render/render_test.cc index 6f364f2ce85..c07b13c1a7a 100644 --- a/xff/presentation/render/render_test.cc +++ b/xff/presentation/render/render_test.cc @@ -69,6 +69,40 @@ TEST_F(RenderTest, PlainAppendsNewline) { EXPECT_THAT(Renderer(Format::kPlain).Record("a/b/c"), "a/b/c\n"); } +TEST_F(RenderTest, PlainRecordsPreserveBytesAcrossStorageAndEscapeGrowthBoundaries) { + constexpr auto kLengths = std::to_array({0, 1, 15, 22, 23, 31, 32, 63, 64, 4'096}); + for (const auto length : kLengths) { + SCOPED_TRACE(length); + const std::string prefix(length, 'x'); + const std::string path = prefix + "\n\\\t"; + EXPECT_THAT(Renderer(Format::kPlain).Record(path), EqualsText(path + "\n")); + EXPECT_THAT(Renderer(Format::kPlain).Record(path, "1;34"), EqualsText("\x1b[1;34m" + path + "\x1b[0m\n")); + EXPECT_THAT( + Renderer(Format::kPlain, PathEncoding::kEscape).Record(path, "1;34"), + EqualsText("\x1b[1;34m" + prefix + "\\n\\\\\\t\x1b[0m\n")); + } +} + +TEST_F(RenderTest, CEscapingCopiesPrintableRunsAroundAdjacentControls) { + constexpr auto kLengths = std::to_array({0, 1, 31, 4'096}); + const std::string controls("\0\t\n\r\\\x1F\x7F", 7); + constexpr std::string_view kEscaped = R"(\x00\t\n\r\\\x1F\x7F)"; + for (const auto length : kLengths) { + SCOPED_TRACE(length); + const std::string run(length, 'x'); + std::string path(controls); + path.append(run).append(controls).append(run).append(controls); + std::string expected(kEscaped); + expected.append(run).append(kEscaped).append(run).append(kEscaped); + EXPECT_THAT(EscapeDisplayText(run), EqualsText(run)); + EXPECT_THAT(EscapeDisplayText(path), EqualsText(expected)); + EXPECT_THAT(Renderer(Format::kPlain, PathEncoding::kEscape).Record(path), EqualsText(expected + "\n")); + Tree tree(false); + tree.Add("root/" + path); + EXPECT_THAT(tree.Render(), EqualsText("root\n`-- " + expected + "\n")); + } +} + TEST_F(RenderTest, NulAppendsNulTerminator) { EXPECT_THAT(Renderer(Format::kNul).Record("a/b/c"), std::string("a/b/c\0", 6)); }