diff --git a/CHANGELOG.md b/CHANGELOG.md index f4416733ba9..aab3febbe73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 0.11.0 +- Reuse budgeted native comparison buffers across chunks while preserving legacy decoder reads. + - Avoid redundant zeroing in native bounded range reads while preserving short-read, EOF and error handling. - Render lexical filename fields and path qualifiers from borrowed input slices instead of temporary path objects. diff --git a/TODO.md b/TODO.md index 694ad8f6101..f65a6d6a413 100644 --- a/TODO.md +++ b/TODO.md @@ -389,8 +389,11 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms [Select native range reads only](docs/benchmarks/host-range-overwrite-20261102.md); preserve VFS and memory-budget boundaries. Cursor zeroing stays unchanged after a repeated Linux ten-worker early-difference regression; revisit it under O07 buffer ownership/reuse. -- [ ] O07-O12: Profile and qualify caller-owned stream buffers, typed reusable directory task slots, - batch ownership, bounded result reuse, active-worker dispatch and direct production evaluation. +- [x] O07: Qualify cursor-owned native buffers, preserving budget lifetimes and legacy streams. + [Native/fallback evidence](docs/benchmarks/cursor-owned-read-buffers-20261102.md) selects one + initial clear plus reuse; reject uninitialized Linux regressions and the first shared fallback loop. +- [ ] O08-O12: Profile and qualify typed reusable directory task slots, batch ownership, + bounded result reuse, active-worker dispatch and direct production evaluation. Preserve existing stage policy while isolating task storage and lifecycle costs. - [ ] O13-O22: Reconcile remaining derived-value handoff, static catalogs, invariant operands, paired streaming compare, reusable stage pools, byte-bounded overlap, archive indexing/admission, @@ -730,6 +733,13 @@ The safety model and directory-scoped permissions are tracked in ## Active engineering priorities +- [ ] Make local clang-tidy header scope resolve Bazel `include_prefix` aliases. The physical + `xff_extras_api/read_source.h` is included as `xff/vfs/read_source.h`, which the current + workspace-only include graph silently misses. Add build-metadata-derived mappings and + regression tests, without a second hand-maintained alias catalog. O07 verification explicitly + resolves those BUILD declarations and lints the affected translation units; keep that evidence + with its report. + 1. **Fuzzing.** The parser-through-evaluator, matcher, template, PHAR, and ASAR targets now have committed seed corpora, semantic invariants, ordinary-CI replay, and automatically discovered bounded CI campaigns. Continue with shard, stronger matcher/template, and regex targets; every diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-linux-fallback.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-linux-fallback.json.gz new file mode 100644 index 00000000000..3e3c9474529 Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-linux-fallback.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-linux-native.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-linux-native.json.gz new file mode 100644 index 00000000000..eced65dbbbf Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-linux-native.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-macos-fallback.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-macos-fallback.json.gz new file mode 100644 index 00000000000..c98100b021e Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-macos-fallback.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-macos-native.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-macos-native.json.gz new file mode 100644 index 00000000000..78db9cf14f1 Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-initial-macos-native.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-provenance.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-provenance.json.gz new file mode 100644 index 00000000000..ac502e2d7e0 Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-provenance.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-linux-fallback.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-linux-fallback.json.gz new file mode 100644 index 00000000000..686e0ed0e5f Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-linux-fallback.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-linux-native.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-linux-native.json.gz new file mode 100644 index 00000000000..2abcccc208f Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-linux-native.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-macos-fallback.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-macos-fallback.json.gz new file mode 100644 index 00000000000..8f8d47fa65e Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-macos-fallback.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-macos-native.json.gz b/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-macos-native.json.gz new file mode 100644 index 00000000000..19fcf410607 Binary files /dev/null and b/docs/benchmarks/cursor-owned-read-buffers-20261102-refined-macos-native.json.gz differ diff --git a/docs/benchmarks/cursor-owned-read-buffers-20261102.md b/docs/benchmarks/cursor-owned-read-buffers-20261102.md new file mode 100644 index 00000000000..8778bc6c2b6 --- /dev/null +++ b/docs/benchmarks/cursor-owned-read-buffers-20261102.md @@ -0,0 +1,104 @@ +# Cursor-owned comparison read buffers (2026-11-02) + +## Decision + +Reuse one 64 KiB buffer per native comparison cursor instead of allocating and clearing an owned string for every chunk. Keep the buffer inside its existing source-budget reservation and destroy it before releasing that reservation. Clear once when allocating the cursor buffer; subsequent reads overwrite only the requested range. Native measurements favor this policy over an entirely uninitialized allocation on Linux. Neither policy relies on precleared bytes for correctness: compare only the initialized byte count returned by the read. + +Add an optional span-based `ReadStream::ReadInto` capability, implemented only by the existing read-only host adapter. The default returns Unimplemented without consuming input, allocating or copying. `HostStream::Read` remains unchanged for other consumers. Memory, archive and decoder streams retain their owning-string path. Select that legacy comparison loop once from the left cursor capability; a mixed legacy-left/native-right pair conservatively uses owned reads for both sides. A native-left/legacy-right pair keeps the legacy bytes only until that comparison step completes. No archive admission or parallelism guard changes. + +The initial shared native/legacy loop is rejected: an unchanged memory-source early-difference control slowed about 29% on macOS. The refined implementation instantiates separate loops at compile time so the legacy path uses local owning chunks as before. Full original and refined source, measurements and slower controls remain below. Do not infer a proven allocator or compiler mechanism from those timing differences. + +## Measurement contract + +Parent `de31e680114aad8d91977c7c05d9bdd0cd4382dd` includes merged #1044/#1045 and the synchronized #1046 range-read change. Both phases compare independently built LLVM 23 release + ThinLTO binaries against that exact parent. Source and executable hashes are in each manifest; the refined selected sources match the retained implementation. No sanitizer or allocation instrumentation enters the timing executables. + +Native fixtures use deterministic nonuniform ASCII bytes with identical, first-byte-different, last-byte-different and different-size cases. Initial qualification measures 1,000 files of 64 KiB and 100 files of 1 MiB at 1/3/10 workers. Refined qualification also adds ten 1 MiB files at those worker counts. The 64 KiB range path and size-only mismatches do not use the new cursor buffer and remain controls. + +Each native case has three sessions, a discarded invocation per variant/session, nine interleaved repetitions and fastest seven retained. Cases and variants use reproducible shuffled order. Every invocation validates every comparison status/path record against an independent exact oracle. Initial plus refined qualification totals 10,800 fresh invocations across both hosts, including discarded warmups. Compare paired session ratios, not absolute times across phases or hosts. + +macOS uses the local M5 Pro, warm APFS and worker allowances without CPU affinity. Linux drogon uses Ryzen 9950X KVM, tmpfs fixtures and child affinity to the first 1/3/10 available virtual CPUs. Builds and diagnostics finish before timing on each host. Commands use `--compare=status --compare-select=all --jobs=N --exact --sort=dir --no-config --no-pager --timezone=UTC LEFT RIGHT -type f`. + +The memory-source fallback control is an independent Google Benchmark release executable: 100/1,000 files, 1/4 workers, small identical and large identical/early-different comparisons. Three variant sessions, three repetitions, 0.02-second warmup and 0.03-second minimum kernel time retain full samples and context. Linux pins each process to its requested kernel worker allowance. These controls deliberately retain their existing 1/4 contract rather than pretend they are native 1/3/10 measurements. + +## Refined native results: 100 files of 1 MiB + +Negative change means faster. Values are median session means; changes are medians of within-session ratios. + +| Host | Case | Workers | Baseline ms | Reuse ms | Change | +| ----- | ------------ | ------: | ----------: | -------: | ------: | +| macos | identical | 1 | 25.560 | 22.154 | -13.81% | +| macos | late | 1 | 25.389 | 21.781 | -14.28% | +| macos | early | 1 | 7.285 | 7.203 | -0.60% | +| macos | size-control | 1 | 4.281 | 4.235 | -1.02% | +| macos | identical | 3 | 12.795 | 11.698 | -8.57% | +| macos | late | 3 | 12.728 | 11.636 | -8.75% | +| macos | early | 3 | 6.197 | 6.208 | -0.09% | +| macos | size-control | 3 | 4.257 | 4.228 | -0.24% | +| macos | identical | 10 | 9.479 | 9.351 | -1.53% | +| macos | late | 10 | 9.277 | 9.115 | -1.40% | +| macos | early | 10 | 6.905 | 6.918 | +0.47% | +| macos | size-control | 10 | 4.268 | 4.266 | -0.92% | +| linux | identical | 1 | 8.844 | 8.220 | -7.57% | +| linux | late | 1 | 8.176 | 7.236 | -10.09% | +| linux | early | 1 | 1.377 | 1.374 | -0.12% | +| linux | size-control | 1 | 0.852 | 0.861 | +0.43% | +| linux | identical | 3 | 3.810 | 3.611 | -5.22% | +| linux | late | 3 | 3.534 | 3.309 | -6.85% | +| linux | early | 3 | 1.285 | 1.297 | +0.66% | +| linux | size-control | 3 | 0.839 | 0.829 | +0.90% | +| linux | identical | 10 | 3.078 | 2.907 | -4.95% | +| linux | late | 10 | 2.903 | 2.776 | -5.90% | +| linux | early | 10 | 1.624 | 1.637 | +0.68% | +| linux | size-control | 10 | 0.867 | 0.852 | -2.62% | + +Complete reads improve most with one or three workers. Early mismatches often stop after one chunk, so they cannot amortize buffer reuse; keep those slower/small results visible. Thread startup and traversal can dominate ten-file and many-worker cases. This does not claim that larger worker counts improve all workloads. + +## Initialization and fallback decisions + +The entirely uninitialized cursor prototype improves some one-worker cases but repeatedly slows Linux ten-worker early differences: +6.88% initially and +8.84% after separating the loops. The selected one-clear policy is +1.32% and +0.68% respectively in the corresponding 100-file cases. Keep the one-time initialization; do not claim that all zeroing should be removed or attribute this result to a proven page-fault explanation. + +| Memory-source control | macOS change | Linux change | +| ---------------------------- | -----------: | -----------: | +| large-early-different/100/1 | -0.12% | +2.17% | +| large-early-different/100/4 | -1.23% | -6.00% | +| large-early-different/1000/1 | +1.15% | +1.29% | +| large-early-different/1000/4 | -4.46% | +2.24% | +| large-identical/100/1 | -0.11% | +1.68% | +| large-identical/100/4 | -0.15% | -5.23% | +| large-identical/1000/1 | -0.28% | +1.43% | +| large-identical/1000/4 | -10.16% | -2.16% | +| small-identical/100/1 | +0.44% | +0.02% | +| small-identical/100/4 | +0.30% | -0.70% | +| small-identical/1000/1 | +0.17% | -0.69% | +| small-identical/1000/4 | +0.11% | -1.20% | + +The initial macOS large-early-different/100/1 control is +29.36%; the refined control is -0.12%. The table preserves remaining small regressions and noisy multi-worker improvements. The fallback algorithm is unchanged, so do not count those fluctuations as an archive or memory-source speedup. Existing archive/decoder integration tests pass, but no new archive throughput improvement is claimed. + +## Allocations, retained storage and counterexamples + +Isolated ASan hooks record counts only, never timing. Comparing two chunks and sixteen chunks uses 13 allocations in the selected implementation on both hosts. The old implementation uses 13 and 43 respectively. Total requested bytes for the sixteen-chunk pair fall from about 2.10 MB to 132 kB. This measures cumulative allocation traffic, not a reduction of the same size in peak RSS. Fixture creation, writes, assertions and warmup occur outside recording. The baseline diagnostic intentionally fails its constant-allocation assertion; the selected implementation passes. Both raw XML/logs are retained. + +Each native pair retains at most two 64 KiB buffers, under the same two source reservations as before. Buffers are cursor-owned, not a global pool or idle per-worker cache; a buffer never outlives its reservation. No extra source-size scan or stat is added. EOF, short non-EOF reads, errors, metadata-sized comparison and left-read-before-right-open order remain unchanged. + +The additive virtual capability is source-compatible with existing decoders. It requires rebuilding statically linked modules and does not promise a stable external binary ABI. There are no new dependencies, flags, locks, execution threads, shared owners or exception wrappers. File access remains inside the authorized VFS adapter; mutations in tests use only the existing temporary-output interface. + +- macos: slowest native paired median +0.93% at (1000, 65536, 10, 'identical'); all small/control cases remain in the raw data. +- macos: build-output executable 4,069,408 -> 4,070,368 bytes (+960), before release stripping/compression. +- linux: slowest native paired median +3.37% at (10, 1048576, 10, 'early'); all small/control cases remain in the raw data. +- linux: build-output executable 12,295,504 -> 12,309,616 bytes (+14,112), before release stripping/compression. + +## Validation + +Both all-extension release suites pass 305/305, with FUSE integration required on Linux. Focused ASan on macOS and MSan on Linux pass the comparison and source tests. Regression tests cover native span caps, zero/short reads, offsets, EOF, preservation of unused buffer tails, mixed capabilities, repeated chunks, invalid returned counts, errors, source-budget exhaustion/release and left-side error precedence. The committed isolated allocation diagnostic verifies that chunk count no longer grows allocation traffic. Raw-pointer and host-I/O policy checks pass. Explicit local lint resolves Bazel include-prefix mappings and checks all 117 affected translation units; TODO records the current scope-selector gap. Final test-only style corrections pass macOS ASan and Linux MSan/allocation checks without changing the measured production sources. Normal commit/push hooks run before publication. + +## Lossless raw evidence + +- [cursor-owned-read-buffers-20261102-initial-macos-native.json.gz](cursor-owned-read-buffers-20261102-initial-macos-native.json.gz): 31,699 bytes; SHA-256 `08c96b5b8e0f692d022283c1f7860ae5e6852e189c57bdf9d42a75ea2af3cad7`. +- [cursor-owned-read-buffers-20261102-initial-linux-native.json.gz](cursor-owned-read-buffers-20261102-initial-linux-native.json.gz): 32,617 bytes; SHA-256 `40fe6ca25e204edab71d222414587a8d07ededd5bc137f964b624c2be7780b05`. +- [cursor-owned-read-buffers-20261102-refined-macos-native.json.gz](cursor-owned-read-buffers-20261102-refined-macos-native.json.gz): 45,704 bytes; SHA-256 `ef358c9e1634a6af8e3a0deef8b01681098a8acbe2539035682ac032aa2967ef`. +- [cursor-owned-read-buffers-20261102-refined-linux-native.json.gz](cursor-owned-read-buffers-20261102-refined-linux-native.json.gz): 46,743 bytes; SHA-256 `0feeaced6e803a199df3e59a1ab182527dfe4f5c28d1b37d748f86a4480b5091`. +- [cursor-owned-read-buffers-20261102-initial-macos-fallback.json.gz](cursor-owned-read-buffers-20261102-initial-macos-fallback.json.gz): 27,451 bytes; SHA-256 `825bedde52ce3a3c5293333bac310d0fc8d4d46aebd49e1b54c05d050242878b`. +- [cursor-owned-read-buffers-20261102-initial-linux-fallback.json.gz](cursor-owned-read-buffers-20261102-initial-linux-fallback.json.gz): 30,145 bytes; SHA-256 `5487789fa7b6651b57d7dcbd63eb55041d575f5eab45423d70f5bfac02f7576d`. +- [cursor-owned-read-buffers-20261102-refined-macos-fallback.json.gz](cursor-owned-read-buffers-20261102-refined-macos-fallback.json.gz): 28,674 bytes; SHA-256 `958775ee4bd76a862bad1580ae54eeb6f425d0dc0811cc22ddec4eae2ce81e0d`. +- [cursor-owned-read-buffers-20261102-refined-linux-fallback.json.gz](cursor-owned-read-buffers-20261102-refined-linux-fallback.json.gz): 30,128 bytes; SHA-256 `e7489710481832ac609eb1c95c9d8ce8921dc7383b4e259ce09a7a39d8a996e1`. +- [cursor-owned-read-buffers-20261102-provenance.json.gz](cursor-owned-read-buffers-20261102-provenance.json.gz): 33,126 bytes; SHA-256 `9d90ce26d65b7efee6ee63d663987ef4beb8a7aee3f5158415a4d35e74afb80f`. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index 978c6f08429..523483a44af 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -2918,8 +2918,9 @@ virtual layers and generic executor hierarchies need an actual lifetime or exten | O21 | Reuse similarity representations | Repeated shingle generation and pairwise verification dominate large document sets | Existing near-duplicate design | Cache reference shingles; exact verification after indexing | | O22 | Reduce output construction churn | Numeric/cell/JSON strings, row insertion and aligned-table retention allocate | Existing output/scope contracts | Append into reusable row buffers; stream machine formats | -O01-O03 have qualified cross-platform reports below. Other entries remain hypotheses until their -individual report records a decision; inspected allocation sites alone do not establish elapsed gains. +O01-O04 and the selected O06/O07 mechanisms have qualified cross-platform reports below. +O05 records two rejected candidates. Other entries remain hypotheses until their individual report +records a decision; inspected allocation sites alone do not establish elapsed gains. ### Small changes first @@ -3183,3 +3184,26 @@ not proven mechanisms. A reusable buffer must remain covered by its applicable r retain worker storage after a cursor budget dies. A generic span-read fallback may add copy/allocation cost for archived/decoded sources. Prefer cursor-scoped reuse first or establish a capability/budget contract and native plus archive controls before introducing persistent worker buffers. + +## O07: cursor-owned native comparison buffers + +The [cursor-buffer qualification report](benchmarks/cursor-owned-read-buffers-20261102.md) selects +one reusable 64 KiB buffer per native cursor, inside its existing reservation. Clear once on +allocation, then overwrite only the initialized bytes reported by the host adapter. The entirely +uninitialized variant repeatedly regresses Linux ten-worker early differences (+6.88%, +8.84%); +keep those controls and avoid claiming a proven allocator/page-fault mechanism. + +An optional native `ReadInto` span capability leaves the existing owned `Read` interface unchanged. +Its unsupported default neither allocates nor advances input. Keep memory/decoder/archive loops +on their original owning-chunk path: the first combined loop slowed one macOS memory control +29.36%, while compile-time-separated loops remove that large regression. Mixed-capability tests, +short reads, EOF, invalid counts, errors, reservation release and left-read precedence remain covered. +No global buffer pool, new thread, lock, dependency, stat, flag or exception wrapper is added. + +The refined 100-file/1 MiB matrix improves complete reads about 14% (one worker) and 9% (three) on +macOS, and 5-10% on Linux. Keep ten-file controls and all slower medians: the worst refined native +case is +0.93% macOS / +3.37% Linux. The isolated ASan counter is diagnostic only: sixteen-chunk +comparisons drop from 43 to 13 allocations, with total requested bytes about 2.10 MB to 132 kB; +this is allocation traffic, not peak RSS. Both fully enabled 305-test suites and focused sanitizers +pass. Initial and refined raw source/binary identities, all 10,800 oracle-checked invocations, +memory fallback kernels and rejected variants are retained in the report artifacts. diff --git a/release-site.json b/release-site.json index 78869b9aa35..5d584777d51 100644 --- a/release-site.json +++ b/release-site.json @@ -64,7 +64,8 @@ "docs/benchmarks/fuzzy-score-workspace-20261102.md": "docs/benchmarks/fuzzy-score-workspace-20261102.html", "docs/benchmarks/lexical-field-views-20261102.md": "docs/benchmarks/lexical-field-views-20261102.html", "docs/benchmarks/field-lookup-controls-20261102.md": "docs/benchmarks/field-lookup-controls-20261102.html", - "docs/benchmarks/host-range-overwrite-20261102.md": "docs/benchmarks/host-range-overwrite-20261102.html" + "docs/benchmarks/host-range-overwrite-20261102.md": "docs/benchmarks/host-range-overwrite-20261102.html", + "docs/benchmarks/cursor-owned-read-buffers-20261102.md": "docs/benchmarks/cursor-owned-read-buffers-20261102.html" }, "generated_html": { "XFF.md": { diff --git a/xff/engine/BUILD.bazel b/xff/engine/BUILD.bazel index b27ae873a11..853cc489716 100644 --- a/xff/engine/BUILD.bazel +++ b/xff/engine/BUILD.bazel @@ -957,9 +957,11 @@ cc_test( ":evaluate_cc", ":expression_execution_cc", ":expression_qualification_cc", + ":parallel_compare_cc", ":walk_cc", "//xff/fuzzy:fuzzy_cc", "//xff/parser:parser_cc", + "//xff/vfs:local_fs_cc", "//xff/vfs:vfs_cc", "@abseil-cpp//absl/base:config", "@abseil-cpp//absl/status", @@ -967,6 +969,7 @@ cc_test( "@abseil-cpp//absl/strings", "@abseil-cpp//absl/time", "@googletest//:gtest_main", + "@mboworks_mbo//mbo/status:status_macros_cc", "@mboworks_mbo//mbo/testing:status_cc", ], ) diff --git a/xff/engine/expression_allocations_test.cc b/xff/engine/expression_allocations_test.cc index 2fe792138dc..06a2e58f80c 100644 --- a/xff/engine/expression_allocations_test.cc +++ b/xff/engine/expression_allocations_test.cc @@ -19,14 +19,18 @@ #include "absl/time/time.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "mbo/status/status_macros.h" #include "mbo/testing/status.h" #include "xff/engine/evaluate.h" #include "xff/engine/expression_execution.h" #include "xff/engine/expression_qualification.h" +#include "xff/engine/parallel_compare.h" #include "xff/engine/walk.h" #include "xff/fuzzy/fuzzy.h" #include "xff/parser/parser.h" #include "xff/vfs/filesystem.h" +#include "xff/vfs/local_fs.h" +#include "xff/vfs/mutations.h" #if defined(ABSL_HAVE_ADDRESS_SANITIZER) # include @@ -35,7 +39,9 @@ namespace xff::engine { namespace { +using ::mbo::testing::IsOkAndHolds; using ::testing::Eq; +using ::testing::Field; using ::testing::Ge; using ::testing::Gt; using ::testing::IsFalse; @@ -192,6 +198,42 @@ TEST_F(AllocationTest, CounterObservesAllocationAndFreeWithoutCountingAssertions EXPECT_THAT(released.counts.frees, Ge(1)); } +TEST_F(AllocationTest, NativeComparisonAllocationsDoNotGrowWithChunkCount) { + const vfs::LocalFs fs; + const auto measure = + [&](std::size_t size) -> absl::StatusOr>> { + MBO_ASSIGN_OR_RETURN(auto left_file, vfs::TemporaryOutput::Create(::testing::TempDir() + "compare-left", {})); + MBO_ASSIGN_OR_RETURN(auto right_file, vfs::TemporaryOutput::Create(::testing::TempDir() + "compare-right", {})); + const std::string bytes(size, 'a'); + MBO_RETURN_IF_ERROR(left_file->Write(bytes)); + MBO_RETURN_IF_ERROR(right_file->Write(bytes)); + const TreeCompareEntry left{ + .path = left_file->Path(), + .metadata = {.type = vfs::FileType::kRegular, .size = size}, + .fs = fs, + }; + const TreeCompareEntry right{ + .path = right_file->Path(), + .metadata = left.metadata, + .fs = fs, + }; + ParallelCompare compare(1); + const std::vector pairs{{.left = left, .right = right}}; + // Warm up one pair outside recording; assertions and fixture writes are never counted. + MBO_RETURN_IF_ERROR(compare.Compare(pairs).front().status()); + return MeasureAllocations([&] { return compare.Compare(pairs).front(); }); + }; + MBO_ASSERT_OK_AND_ASSIGN(const auto short_read, measure(ParallelCompare::kRetainedFileBytes + 1)); + MBO_ASSERT_OK_AND_ASSIGN(const auto long_read, measure(16 * ParallelCompare::kRetainedFileBytes)); + EXPECT_THAT(short_read.value, IsOkAndHolds(Field(&ComparisonResult::same, IsTrue()))); + EXPECT_THAT(long_read.value, IsOkAndHolds(Field(&ComparisonResult::same, IsTrue()))); + EXPECT_THAT(short_read.counts.allocations, Ge(2)); + EXPECT_THAT(long_read.counts.allocations, Eq(short_read.counts.allocations)); + EXPECT_THAT(long_read.counts.requested_bytes, Eq(short_read.counts.requested_bytes)); + RecordCounts("two_chunks", short_read.counts); + RecordCounts("sixteen_chunks", long_read.counts); +} + TEST_F(AllocationTest, ReusedFuzzyWorkspaceAvoidsHotScoreAllocations) { const fuzzy::FzfQuery query("fl"); fuzzy::ScoreWorkspace workspace; diff --git a/xff/engine/parallel_compare.cc b/xff/engine/parallel_compare.cc index bb822d9fe8d..a137c703ded 100644 --- a/xff/engine/parallel_compare.cc +++ b/xff/engine/parallel_compare.cc @@ -4,7 +4,9 @@ #include "xff/engine/parallel_compare.h" #include +#include #include +#include #include #include "mbo/status/status_macros.h" @@ -31,15 +33,53 @@ absl::StatusOr ReadChunk(vfs::ReadStream& stream, std::size_t lengt } struct Cursor { + using Buffer = std::array; + + // Destroy backing storage before releasing the reservation that covers it. vfs::ReadBudget::Reservation reservation; std::unique_ptr stream; + std::unique_ptr buffer; + std::string legacy_bytes; + bool direct_read = false; + + void ReleaseLegacyBytes() { std::string{}.swap(legacy_bytes); } + + absl::StatusOr Read(std::size_t length) { + if (!direct_read) { + MBO_ASSIGN_OR_RETURN(legacy_bytes, ReadChunk(*stream, length)); + return legacy_bytes; + } + if (!buffer) { + // Reuse this allocation for every chunk. The one initial clear is a measured policy: + // removing it regresses Linux early-difference workloads with many workers. + buffer = std::make_unique(); + } + std::size_t total = 0; + while (total < length) { + const auto remaining = std::span(*buffer).first(length).subspan(total); + MBO_ASSIGN_OR_RETURN(const auto count, stream->ReadInto(remaining)); + if (count > remaining.size()) { + return absl::DataLossError("cursor returned more bytes than requested"); + } + if (count == 0) { + break; + } + total += count; + } + return std::string_view(buffer->data(), total); + } }; absl::StatusOr OpenEntry(const TreeCompareEntry& entry) { MBO_ASSIGN_OR_RETURN(const auto source, entry.fs->ContentSource(entry.path)); MBO_ASSIGN_OR_RETURN(auto reservation, source->Budget()->Reserve(ParallelCompare::kRetainedFileBytes)); MBO_ASSIGN_OR_RETURN(auto stream, source->Open()); - return Cursor{.reservation = std::move(reservation), .stream = std::move(stream)}; + const bool direct_read = stream->SupportsReadInto(); + return Cursor{ + .reservation = std::move(reservation), + .stream = std::move(stream), + .direct_read = direct_read, + }; } absl::StatusOr CompareSmall( @@ -60,30 +100,38 @@ absl::StatusOr CompareSmall( return result; } -absl::StatusOr CompareRegular( - const TreeCompareEntry& left, - const TreeCompareEntry& right, - bool retain) { - if (left.metadata.size != right.metadata.size) { - return ComparisonResult{}; - } - if (left.metadata.size == 0) { - return ComparisonResult{.same = true}; - } - if (left.metadata.size <= ParallelCompare::kRetainedFileBytes) { - return CompareSmall(left, right, retain); +template +auto ReadComparisonChunk(Cursor& cursor, std::size_t length) { + if constexpr (kCallerOwned) { + return cursor.Read(length); + } else { + return ReadChunk(*cursor.stream, length); } - MBO_ASSIGN_OR_RETURN(auto left_stream, OpenEntry(left)); +} + +template +absl::StatusOr CompareStreams( + Cursor& left_stream, + const TreeCompareEntry& right, + std::uint64_t size) { std::optional right_stream; constexpr std::size_t kChunk = ParallelCompare::kRetainedFileBytes; - for (std::uint64_t offset = 0; offset < left.metadata.size; offset += kChunk) { - const auto length = static_cast(std::min(kChunk, left.metadata.size - offset)); - MBO_ASSIGN_OR_RETURN(const auto left_bytes, ReadChunk(*left_stream.stream, length)); + for (std::uint64_t offset = 0; offset < size; offset += kChunk) { + const auto length = static_cast(std::min(kChunk, size - offset)); + // Legacy strings previously died at the end of each iteration. Release both before the + // next read so fallback decoders do not retain an extra, unreserved previous chunk. + if constexpr (kCallerOwned) { + left_stream.ReleaseLegacyBytes(); + if (right_stream) { + right_stream->ReleaseLegacyBytes(); + } + } + MBO_ASSIGN_OR_RETURN(const auto left_bytes, ReadComparisonChunk(left_stream, length)); if (!right_stream) { MBO_ASSIGN_OR_RETURN(auto cursor, OpenEntry(right)); right_stream.emplace(std::move(cursor)); } - MBO_ASSIGN_OR_RETURN(const auto right_bytes, ReadChunk(*right_stream->stream, length)); + MBO_ASSIGN_OR_RETURN(const auto right_bytes, ReadComparisonChunk(*right_stream, length)); if (left_bytes != right_bytes) { return ComparisonResult{}; } @@ -94,6 +142,26 @@ absl::StatusOr CompareRegular( return ComparisonResult{.same = true}; } +absl::StatusOr CompareRegular( + const TreeCompareEntry& left, + const TreeCompareEntry& right, + bool retain) { + if (left.metadata.size != right.metadata.size) { + return ComparisonResult{}; + } + if (left.metadata.size == 0) { + return ComparisonResult{.same = true}; + } + if (left.metadata.size <= ParallelCompare::kRetainedFileBytes) { + return CompareSmall(left, right, retain); + } + MBO_ASSIGN_OR_RETURN(auto left_stream, OpenEntry(left)); + // Keep decoded/memory-source chunks as local owning values, exactly as before. The native + // buffer path must not add per-chunk ownership or dispatch work to legacy streams. + return left_stream.direct_read ? CompareStreams(left_stream, right, left.metadata.size) + : CompareStreams(left_stream, right, left.metadata.size); +} + absl::StatusOr ComparePair(const ComparisonPair& pair, bool retain) { if (!pair.left || !pair.right || pair.left->metadata.type != pair.right->metadata.type) { return ComparisonResult{}; diff --git a/xff/engine/parallel_compare_test.cc b/xff/engine/parallel_compare_test.cc index 18cf5519329..6ca2f250768 100644 --- a/xff/engine/parallel_compare_test.cc +++ b/xff/engine/parallel_compare_test.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -42,35 +43,64 @@ using ::testing::SizeIs; class ShortSource final : public vfs::ReadSource { public: + explicit ShortSource(std::shared_ptr budget = std::make_shared()) + : ReadSource(std::move(budget)) {} + std::string bytes = "abcdef"; bool fail_open = false; std::size_t fail_at = 100; + bool direct_read = false; + bool oversized_read = false; + std::size_t maximum_chunk = 2; absl::StatusOr> Open() const override { if (fail_open) { return absl::PermissionDeniedError("cursor open"); } - return std::make_unique(bytes, fail_at); + return std::make_unique(bytes, fail_at, direct_read, oversized_read, maximum_chunk); } private: class Stream final : public vfs::ReadStream { public: - Stream(std::string bytes, std::size_t fail_at) : bytes_(std::move(bytes)), fail_at_(fail_at) {} + Stream(std::string bytes, std::size_t fail_at, bool direct_read, bool oversized_read, std::size_t maximum_chunk) + : bytes_(std::move(bytes)), + fail_at_(fail_at), + direct_read_(direct_read), + oversized_read_(oversized_read), + maximum_chunk_(maximum_chunk) {} absl::StatusOr Read(std::size_t maximum) override { if (offset_ >= fail_at_) { return absl::DataLossError("cursor read"); } - const auto chunk = std::string_view(bytes_).substr(offset_, std::min(std::size_t{2}, maximum)); + const auto chunk = std::string_view(bytes_).substr(offset_, std::min(maximum_chunk_, maximum)); offset_ += chunk.size(); return std::string(chunk); } + bool SupportsReadInto() const noexcept override { return direct_read_; } + + absl::StatusOr ReadInto(std::span buffer) override { + if (oversized_read_) { + return buffer.size() + 1; + } + if (offset_ >= fail_at_) { + return absl::DataLossError("cursor read"); + } + const auto chunk = std::string_view(bytes_).substr(offset_, std::min(maximum_chunk_, buffer.size())); + std::ranges::copy(chunk, buffer.begin()); + offset_ += chunk.size(); + return chunk.size(); + } + private: std::string bytes_; std::size_t fail_at_; std::size_t offset_ = 0; + bool direct_read_; + bool oversized_read_; + std::size_t maximum_chunk_; }; }; @@ -244,6 +274,62 @@ TEST_F(ParallelCompareTest, CursorFailuresRemainErrors) { EXPECT_THAT(compare.Compare({{.left = left, .right = right}}).front(), StatusIs(absl::StatusCode::kDataLoss)); } +TEST_F(ParallelCompareTest, CallerOwnedReadsHandleShortChunksEofAndLegacyPeers) { + const auto source = std::make_shared(); + source->direct_read = true; + fs.source_override = source; + left.metadata.size = right.metadata.size = 262'144; + ParallelCompare compare(1); + EXPECT_THAT( + compare.Compare({{.left = left, .right = right}}).front(), + IsOkAndHolds(Field(&ComparisonResult::same, IsTrue()))); + source->bytes.clear(); + EXPECT_THAT( + compare.Compare({{.left = left, .right = right}}).front(), + IsOkAndHolds(Field(&ComparisonResult::same, IsTrue()))); + + CompareFs other_fs; + right.fs.set_ref(other_fs); + source->bytes.assign(200'017, 'a'); + source->fail_at = source->bytes.size() + 1; + source->maximum_chunk = 17'003; + other_fs.files.at("right") = source->bytes; + EXPECT_THAT( + compare.Compare({{.left = left, .right = right}}).front(), + IsOkAndHolds(Field(&ComparisonResult::same, IsTrue()))); + other_fs.files.at("right").back() = 'b'; + EXPECT_THAT( + compare.Compare({{.left = left, .right = right}}).front(), + IsOkAndHolds(Field(&ComparisonResult::same, IsFalse()))); + // Reverse the capability mix: the left stream now uses the legacy owned-string path. + const auto right_source = std::make_shared(); + right_source->bytes = source->bytes; + right_source->fail_at = source->fail_at; + right_source->maximum_chunk = source->maximum_chunk; + right_source->direct_read = true; + source->direct_read = false; + other_fs.source_override = right_source; + EXPECT_THAT( + compare.Compare({{.left = left, .right = right}}).front(), + IsOkAndHolds(Field(&ComparisonResult::same, IsTrue()))); +} + +TEST_F(ParallelCompareTest, CallerOwnedErrorsAndInvalidCountsDoNotOpenRightSide) { + const auto source = std::make_shared(); + source->direct_read = true; + fs.source_override = source; + const CompareFs other_fs; + right.fs.set_ref(other_fs); + left.metadata.size = right.metadata.size = 262'144; + ParallelCompare compare(1); + source->fail_at = 2; + EXPECT_THAT(compare.Compare({{.left = left, .right = right}}).front(), StatusIs(absl::StatusCode::kDataLoss)); + EXPECT_THAT(other_fs.sources.load(), Eq(0)); + source->oversized_read = true; + EXPECT_THAT(compare.Compare({{.left = left, .right = right}}).front(), StatusIs(absl::StatusCode::kDataLoss)); + EXPECT_THAT(other_fs.sources.load(), Eq(0)); +} + TEST_F(ParallelCompareTest, EmptyFilesNeedNoCursorAndOwnedSourcesStaySerial) { left.metadata.size = right.metadata.size = 0; ParallelCompare compare(4); @@ -358,6 +444,29 @@ TEST_F(ParallelCompareTest, CursorBuffersRespectAndReleaseSourceBudget) { EXPECT_THAT(enough->MemoryUsed(), Eq(0)); } +TEST_F(ParallelCompareTest, CallerOwnedBuffersReleaseReservationsOnSuccessAndErrors) { + left.metadata.size = right.metadata.size = 262'144; + ParallelCompare compare(1); + const auto insufficient = std::make_shared(ParallelCompare::kRetainedFileBytes); + const auto source = std::make_shared(insufficient); + source->direct_read = true; + fs.source_override = source; + EXPECT_THAT( + compare.Compare({{.left = left, .right = right}}).front(), StatusIs(absl::StatusCode::kResourceExhausted)); + EXPECT_THAT(insufficient->MemoryUsed(), Eq(0)); + const auto enough = std::make_shared(2 * ParallelCompare::kRetainedFileBytes); + const auto complete = std::make_shared(enough); + complete->direct_read = true; + fs.source_override = complete; + EXPECT_THAT( + compare.Compare({{.left = left, .right = right}}).front(), + IsOkAndHolds(Field(&ComparisonResult::same, IsTrue()))); + EXPECT_THAT(enough->MemoryUsed(), Eq(0)); + complete->fail_at = 2; + EXPECT_THAT(compare.Compare({{.left = left, .right = right}}).front(), StatusIs(absl::StatusCode::kDataLoss)); + EXPECT_THAT(enough->MemoryUsed(), Eq(0)); +} + TEST_F(ParallelCompareTest, SymlinkErrorsRemainErrors) { left.metadata.type = right.metadata.type = vfs::FileType::kSymlink; ParallelCompare compare(1); diff --git a/xff_extras_api/read_source.cc b/xff_extras_api/read_source.cc index 50b3d22c530..2adc21bff26 100644 --- a/xff_extras_api/read_source.cc +++ b/xff_extras_api/read_source.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -77,6 +78,17 @@ class HostStream final : public ReadStream { return result; } + bool SupportsReadInto() const noexcept override { return true; } + + // XFF_HOST_IO: reads into the cursor owner's reserved storage using its read-only descriptor. + absl::StatusOr ReadInto(std::span buffer) override { + const auto count = ::read(fd_, buffer.data(), std::min(buffer.size(), 64UZ * 1'024)); + if (count < 0) { + return absl::DataLossError(absl::ErrnoToStatus(errno, "reading container source failed").message()); + } + return static_cast(count); + } + private: int fd_; }; diff --git a/xff_extras_api/read_source.h b/xff_extras_api/read_source.h index 5b1481a7b0c..49be458826f 100644 --- a/xff_extras_api/read_source.h +++ b/xff_extras_api/read_source.h @@ -8,10 +8,12 @@ #include #include #include +#include #include #include #include +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "xff/vfs/read_budget.h" @@ -23,6 +25,15 @@ class ReadStream { public: virtual ~ReadStream() = default; virtual absl::StatusOr Read(std::size_t max_bytes) = 0; + + // Optional allocation-free reads into caller-owned storage. Only [0, returned count) is + // initialized; zero means EOF and a short read need not mean EOF. The default never reads + // or allocates, so sequential decoders keep their existing owned Read path. + virtual bool SupportsReadInto() const noexcept { return false; } + + virtual absl::StatusOr ReadInto(std::span) { + return absl::UnimplementedError("cursor does not support caller-owned read buffers"); + } }; // An immutable, restartable source. Open can be called concurrently; each cursor owns everything diff --git a/xff_extras_api/read_source_test.cc b/xff_extras_api/read_source_test.cc index a85efb30fa7..a4eeb3ca3fb 100644 --- a/xff_extras_api/read_source_test.cc +++ b/xff_extras_api/read_source_test.cc @@ -5,8 +5,10 @@ #include #include +#include #include #include +#include #include "absl/status/status.h" #include "gmock/gmock.h" @@ -24,10 +26,54 @@ using ::mbo::testing::StatusIs; using ::testing::Eq; using ::testing::Field; using ::testing::IsEmpty; +using ::testing::IsFalse; +using ::testing::IsTrue; using ::testing::Optional; struct ReadSourceTest : ::testing::Test {}; +TEST_F(ReadSourceTest, UnsupportedReadIntoNeitherConsumesNorCopiesMemorySource) { + MBO_ASSERT_OK_AND_ASSIGN(auto stream, MemoryReadSource("abcdef")->Open()); + EXPECT_THAT(stream->SupportsReadInto(), IsFalse()); + std::string buffer = "unchanged"; + EXPECT_THAT(stream->ReadInto(std::span(buffer)), StatusIs(absl::StatusCode::kUnimplemented)); + EXPECT_THAT(buffer, EqualsText("unchanged")); + EXPECT_THAT(stream->Read(6), IsOkAndHolds(EqualsText("abcdef"))); +} + +TEST_F(ReadSourceTest, HostReadIntoCapsChunksPreservesTailAndSharesCursorPosition) { + MBO_ASSERT_OK_AND_ASSIGN(auto directory, TemporaryDirectory::Create(::testing::TempDir() + "read-into", {})); + MBO_ASSERT_OK_AND_ASSIGN(auto output, TemporaryOutput::Create(directory->Path() + "/file", {})); + std::string bytes((2UZ * 64 * 1'024) + 17, 'a'); + for (std::size_t at = 0; at < bytes.size(); ++at) { + bytes.at(at) = static_cast('a' + (at % 26)); + } + ASSERT_THAT(output->Write(bytes), IsOk()); + const auto source = HostReadSource(output->Path()); + MBO_ASSERT_OK_AND_ASSIGN(auto stream, source->Open()); + EXPECT_THAT(stream->SupportsReadInto(), IsTrue()); + std::vector storage(70'000, '!'); + const std::span buffer(storage); + EXPECT_THAT(stream->ReadInto(buffer.first(0)), IsOkAndHolds(Eq(0))); + EXPECT_THAT(stream->ReadInto(buffer), IsOkAndHolds(Eq(64UZ * 1'024))); + EXPECT_THAT(std::string_view(buffer.data(), 64UZ * 1'024), EqualsText(bytes.substr(0, 64UZ * 1'024))); + const auto unused = buffer.subspan(64UZ * 1'024); + EXPECT_THAT( + std::string_view(unused.data(), unused.size()), EqualsText(std::string(buffer.size() - (64UZ * 1'024), '!'))); + EXPECT_THAT(stream->Read(3), IsOkAndHolds(EqualsText(bytes.substr(64UZ * 1'024, 3)))); + const auto remaining = bytes.substr((64UZ * 1'024) + 3); + EXPECT_THAT(stream->ReadInto(buffer), IsOkAndHolds(Eq(64UZ * 1'024))); + EXPECT_THAT(std::string_view(buffer.data(), 64UZ * 1'024), EqualsText(remaining.substr(0, 64UZ * 1'024))); + EXPECT_THAT(stream->ReadInto(buffer), IsOkAndHolds(Eq(14))); + EXPECT_THAT(std::string_view(buffer.data(), 14), EqualsText(bytes.substr(bytes.size() - 14))); + EXPECT_THAT(stream->ReadInto(buffer), IsOkAndHolds(Eq(0))); + MBO_ASSERT_OK_AND_ASSIGN(auto partial, source->OpenAt(bytes.size() - 3)); + EXPECT_THAT(partial->ReadInto(buffer), IsOkAndHolds(Eq(3))); + EXPECT_THAT(std::string_view(buffer.data(), 3), EqualsText(bytes.substr(bytes.size() - 3))); + MBO_ASSERT_OK_AND_ASSIGN(auto unreadable, HostReadSource(directory->Path())->Open()); + EXPECT_THAT(unreadable->ReadInto(buffer), StatusIs(absl::StatusCode::kDataLoss)); +} + TEST_F(ReadSourceTest, HostIdentityIsAbsentForVirtualSources) { EXPECT_THAT(MemoryReadSource("content")->HostPath(), Eq(std::nullopt)); EXPECT_THAT(HostReadSource("/host/file")->HostPath(), Optional(Eq("/host/file")));