diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a060d61283..35154c7a2e 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)\.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)\.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 4c0b53ab38..24071bf6c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ # 0.11.0 +- Transfer validated rg output directly into matcher results, reusing retained buffers while preserving + native callback boundaries, binary suppression, and read-error behavior. + - Append structured `-printf` field values directly to the output buffer while preserving owning renders and content-snapshot lifetimes. diff --git a/TODO.md b/TODO.md index 7ad4654e58..fbb0a6612d 100644 --- a/TODO.md +++ b/TODO.md @@ -682,6 +682,12 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms preserve bounded view lifetimes, sparse/zero metrics, full outputs and unchanged-codec controls on both hosts. Retain slower cases and confirmation in [the direct-cell report](docs/benchmarks/direct-summary-cells-20261104.md). +- [x] O22c2h: Integrate qualified owned transactional rg records directly into matcher results. + Native-in-place passes both-host 323-target release and focused ASan validation; + memory large-output cohorts improve 12.36% Mac/4.32% Linux while slower controls + remain documented. Preserve read/binary validation, callback boundaries and reused + capacity. Require current-head PR CI; O18 byte-bounded overlap remains separate. See + [the complete experiment](docs/benchmarks/transactional-rg-records-20261107.md). - [ ] O22c2: Qualify remaining reusable structured cell/row storage 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. diff --git a/docs/benchmarks/transactional-rg-records-20261107.json.gz b/docs/benchmarks/transactional-rg-records-20261107.json.gz new file mode 100644 index 0000000000..69669f5fbf Binary files /dev/null and b/docs/benchmarks/transactional-rg-records-20261107.json.gz differ diff --git a/docs/benchmarks/transactional-rg-records-20261107.md b/docs/benchmarks/transactional-rg-records-20261107.md new file mode 100644 index 0000000000..40cda14145 --- /dev/null +++ b/docs/benchmarks/transactional-rg-records-20261107.md @@ -0,0 +1,161 @@ +# Owned transactional rg output, 2026-11-07 + +The selected variant passes owned rendered records directly to the matcher result, +removing three allocations per entry in the isolated 64-entry fixtures. Large-output +cohort times improve by 12.36% on Mac and 4.32% on Linux in the in-memory matrix. +Ordinary whole-command rg-line medians are effectively unchanged. This is a targeted +output improvement with retained slower cases, not a general search speedup. + +## Mechanism and semantics + +Previously, line rendering accumulated a complete transaction plus record-end offsets, +replayed it into another per-file string, and copied that string into the parallel +result. `RenderRgOutput` now returns an owned string and selection state. A worker +moves its reusable result allocation into the renderer and takes the transaction back. +Ordinary rg line output needs neither record-end indices nor replay. Native grep +retains its original local buffer, boundaries, frozen sink and bool/status result. +Reduced and multiline rg output keep the validated capture path. + +Publication still waits for the complete read and binary checks. A late read failure +or binary marker discards the file's transaction. Heading/filename and sink state +are restored on errors. Existing result-capacity retention limits remain; no worker +count, batch size, CLI flag, VFS operation or archive barrier changes. The preceding +helpers were already inlined under production LTO, so removing surviving function +calls is not the claimed mechanism. + +## Qualification protocol + +Baseline is `c95140674254c371844f1fab27a64951a80dac55` (#1119). Subsequent base changes +through #1125 affect documentation rather than this implementation. Recorded source +hashes and unstripped executable hashes identify each measured participant. Both hosts +use explicit `--config=clang_release`; no sanitizer executable or duration contributes +to timing. Existing production benchmark fixtures are unchanged. Mac measurements are +unpinned on the local M5 Pro; Linux uses one pinned logical CPU per physical core. +No build, lint or sanitizer work runs concurrently with timing on the same host. + +The memory matrix has 128 cases per host and variant: RE2/PCRE2, eight tasks, +10/100/1,000/10,000 entries, and 1/4 workers. Three counterbalanced sessions each +contain nine raw repetitions; retain the fastest seven and compute their mean. +Each case's reported change is the median of the three paired session changes; +the cohort statistic below is the median across those case changes. A cohort is +not weighted by elapsed time, bytes or invocation frequency. Positive is slower. + +The native matrix has 124 cases per host and variant. It covers flat and many-directory +trees, listing/name/metadata/native RE2/rg-file/native-line/rg-line commands, the same +four entry counts and worker settings, plus 10/100/1,000-file output fixtures. Each +native output file has 32 4KiB lines; memory large-output files have one giant 128KiB +line. Those are distinct workloads. Files are warm APFS on Mac and tmpfs on Linux. + +Native participants rotate across baseline, an exact same-binary baseline repeat, +and candidate. Each of three sessions has nine measured rounds, retaining seven, +after discarded warmups. A native launcher excludes Python process startup from +measured time; process launch and the complete xff invocation remain included. +Every invocation must match a fixture-derived complete output oracle. All oracles +pass. Raw wall time, CPU and RSS observations are retained, without a native RSS claim. + +## Selected variant: memory matrix + +| Task | Mac median change | Linux median change | +| :----------- | ----------------: | ------------------: | +| traversal | +0.15% | +0.02% | +| filter | +0.51% | -1.04% | +| native-lines | +0.64% | +3.18% | +| rg-lines | -1.49% | -0.26% | +| rg-files | +0.60% | +0.37% | +| rg-counts | +0.39% | +0.86% | +| rg-quiet | +0.99% | +1.76% | +| large-output | -12.36% | -4.32% | + +The Linux native-line memory cohort remains 3.18% slower. Retain the Mac PCRE2 +native-lines/10,000/4 case at +6.63%, Mac RE2 quiet/10,000/1 at +5.49%, Linux PCRE2 +count/100/4 at +8.53%, and Linux PCRE2 native-lines/1,000/1 at +4.57%. Allocation +counts do not explain every timing difference; there is no measured causal claim +that these are capacity, placement or unrelated-system effects. + +## Selected variant: whole-command controls + +| Task | Mac candidate | Mac same-binary repeat | Linux candidate | Linux same-binary repeat | +| :---------------- | ------------: | ---------------------: | --------------: | -----------------------: | +| files-exact | +0.04% | -0.59% | +0.76% | +0.32% | +| name-exact | +0.36% | -0.18% | +1.51% | -0.22% | +| metadata | +0.78% | -0.87% | -0.07% | +0.16% | +| native-filter-RE2 | +0.44% | -0.47% | +0.67% | -0.26% | +| rg-filter | +0.17% | -0.59% | +0.48% | -0.11% | +| native-lines | +0.16% | -0.39% | +0.64% | +0.27% | +| rg-lines | -0.23% | -0.31% | +0.00% | -0.34% | + +These cohorts include the separate output fixtures where applicable. Large native +rg-output cases improve by up to 11.35% on Mac and 50.56% on Linux; these are individual +case improvements, not cohort or general CLI claims. Preserve Linux flat10/rg-lines/4 +at +4.50% (repeat -1.17%), many10,000/files/4 at +4.42% (repeat +0.45%), and Mac +flat100/files/1 at +2.06% (repeat -1.45%). The complete per-case and per-session +population, including every slower cell, is in the compressed evidence. + +## Allocation traffic and image sizes + +The diagnostic uses 64 entries, one inline production matcher and two recorded +repetitions. Fixture setup, warmup and assertions stay outside recording. Counts +are identical in both recorded repetitions. This is an allocation diagnostic under +ASan, not timing evidence or an RSS measurement. + +| Host | Padding bytes | Allocations before | Allocations after | Requested bytes before | Requested bytes after | Retained result capacity before/after | +| :---- | ------------: | -----------------: | ----------------: | ---------------------: | --------------------: | ------------------------------------: | +| Mac | 1,024 | 704 | 512 | 349,696 | 217,088 | 65,984 | +| Linux | 1,024 | 704 | 512 | 350,720 | 217,088 | 66,496 | +| Mac | 131,072 | 960 | 768 | 62,939,136 | 46,160,384 | 8,389,056 | +| Linux | 131,072 | 960 | 768 | 62,942,720 | 46,162,944 | 8,389,568 | + +The diagnostic removes three allocations per entry. Retained capacities are unchanged +within each host. The two allocator implementations have different capacities; do not +attribute timing differences to increased result retention from these fixtures. + +Unstripped lean CLI images grow from 4,193,792 to 4,196,608 bytes on Mac (+2,816), and +12,687,760 to 12,699,000 on Linux (+11,240). These are frozen experiment images, not +sizes of full release distributions. + +## Other preserved variants + +Transfer-only ownership moved the outer transaction but kept inner replay. Its memory +rg-line cohort changes were +0.47% Mac/+0.66% Linux; large-output +6.77%/-2.11%. +Mac RE2 large-output/10/1 was +23.30% and PCRE2 large-output/10,000/4 +21.85%. +It is unselected; fewer allocations alone did not qualify it. + +The intermediate single-transaction variant also returned a transaction/index aggregate +for native grep replay. Memory large output improved 11.73% Mac/4.20% Linux, but native +lines cost 0.98%/3.50%, and Mac RE2 rg-files/10/4 cost 10.69%. Native rg-line medians +were -0.39% Mac/+0.43% Linux, with same-binary repeats -0.43%/-0.15%. It remains +unselected; the final specialization preserves native's original local publication. +All three source variants, raw populations, controls and available validation logs +remain separately named in the evidence. No unchanged candidate was retimed until favorable. + +## Correctness and integration limits + +Both hosts pass optimized `rg_test`, `parallel_match_test` and `evaluate_test`, plus +canonical Clang+ASan runs of those three targets and `expression_allocations_test`. +Both pass all 323 core and registered-extension targets with `clang_release` and +`xff_docs`; Linux requires the real FUSE test rather than silently omitting it. +The earlier lean-root 253-passed/10-skipped run is separate and is not described as +complete extension validation. Command arrays and complete logs are retained. + +Regression cases in this change cover native callback boundaries/context separators, +callbacks replacing the sink, late read failures, headings and sink restoration, +late binary suppression, nonmatching scratch, and bounded reused result capacity. +Existing rg mode tests cover reduced/multiline output and parallel behavior. Final +integration on current main still requires the PR's own required CI, including +coverage, GCC, CI clang-tidy and sanitizers. Development results are not substituted +for those checks. The integration on `d8cf61cccd4b0efed5058bf6fa45228e47162b48` +(#1126) passes the three optimized semantic targets on Mac. The five C++ source/test +files remain byte-identical to the qualified experiment; this pre-publication check +is separate from the eventual pushed-head PR CI. + +This completes the narrow O22c2h record-ownership experiment. O18 still needs global +byte admission/backpressure, ordered per-entry completion, cancellation/draining, +a head-credit reservation and an explicit oversized transactional-output contract. +This change does not bound a single file's output or overlap pipeline stages. + +## Retained evidence + +[`transactional-rg-records-20261107.json.gz`](transactional-rg-records-20261107.json.gz) is 5,079,513 bytes, SHA-256 +`f32ec19cd3af284576a8f6af7e84fbedbf10dee48b044bfdc05909e0ffc7c466`. It contains 283 exact UTF-8 source/raw/log records with per-record byte counts and hashes. +Executables remain in the local/remote experiment directories; image identities and sizes +are included. The source of the native timing launcher and fixture generators is included. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index a90ea21cbd..cffa65b430 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -4750,3 +4750,49 @@ Its exact post-merge main Test run 42276661524 passed both operating systems, AS TSan, GCC, coverage, fuzzing, changed-file lint and policy checks. Close the stale create-PR instruction without removing the earlier noisy Mac populations or extending private benchmark image claims to a different public integration image. + +### O18 audit: preserve per-file output transactions before adding overlap + +`ParallelMatch::Match` currently waits for every result before returning a batch. +Simple double buffering cannot make rendered output byte-bounded: a single file's +matches can exceed any fixed limit. `RenderGrepLines` retains transactional rendered bytes until read/binary validation. +O22c2h returns that owned transaction directly to the matcher result, reusing its old +capacity and avoiding rg record-end indices, replay and the outer record copy. +Native grep retains its original local buffer and callback boundaries. Reduced and +multiline rg paths retain validated capture. This does not establish pipeline +overlap, output backpressure or a global memory cap. + +O18 needs ordered per-entry completion rather than wait-for-all if producers may +block on output credit. Later entries must not consume the credit needed to finish +the head entry. Charge actual retained input text, slot/result capacities and output +allocations before admission; returning a completed string to a full queue is too +late. Worker-held completed strings are still retained output. Arbitrarily large +head transactions need an explicit oversize or transactional-storage contract, +without truncation, hidden extra reads or loss of late-error/binary suppression. +Active source/decoder memory is a separate bound, not free queue storage. + +Preserve coordinator publication, traversal-error ordering, effect/capture/deferred +barriers, serial archive fallback and owner draining. Cancellation must wake both +blocked publishers and consumers before joining persistent workers. Qualify delayed +heads with full later records, giant/no-output files, preparation/read failures, +root/mount lifetimes, actual byte peaks, first-output latency and small-case costs on +both hosts. The complete O18 pipeline and the JC storage/admission gates remain open. + +### O22c2h: transfer the original validated rg transaction + +[The complete report](benchmarks/transactional-rg-records-20261107.md) retains three +source variants, every measured cell and the copied-baseline controls. The selected +native-in-place variant removes three allocations per entry in the isolated +64-entry diagnostic without increasing retained result capacity. Production memory +large-output cohorts improve 12.36% on Mac and 4.32% on Linux; native whole-command +rg-line medians change -0.23% and +0.00%, respectively. Native large-output gains up +to 11.35%/50.56% are case-specific. This is not a general startup or search speedup. + +Retain Linux's +3.18% native-line memory cohort, +8.53% PCRE2 count/100/4 cell and +Mac's +6.63% PCRE2 native-line/10,000/4 cell. Transfer-only and aggregate-return +intermediate variants remain unselected with their complete negative evidence. +Both hosts pass all 323 documented-surface/core/registered-extension release targets +and four focused canonical ASan targets. These development results require the +current integration PR's own coverage, sanitizer, GCC and CI clang-tidy checks. +O18's byte admission, ordered completion, oversized output and cancellation work +remains open. diff --git a/release-site.json b/release-site.json index 26f99e5557..5d681b2432 100644 --- a/release-site.json +++ b/release-site.json @@ -134,7 +134,8 @@ "docs/benchmarks/cli-stream-sync-audit-20261105.md": "docs/benchmarks/cli-stream-sync-audit-20261105.html", "docs/benchmarks/archive-member-keys-20261105.md": "docs/benchmarks/archive-member-keys-20261105.html", "docs/benchmarks/shared-stage-workers-20261107.md": "docs/benchmarks/shared-stage-workers-20261107.html", - "docs/benchmarks/archive-index-insertion-20261107.md": "docs/benchmarks/archive-index-insertion-20261107.html" + "docs/benchmarks/archive-index-insertion-20261107.md": "docs/benchmarks/archive-index-insertion-20261107.html", + "docs/benchmarks/transactional-rg-records-20261107.md": "docs/benchmarks/transactional-rg-records-20261107.html" }, "generated_html": { "XFF.md": { @@ -155,5 +156,8 @@ "label": "Benchmarks", "href": "/{repo}/benchmarks/tag/{version}/" } - ] + ], + "files": { + "docs/benchmarks/transactional-rg-records-20261107.json.gz": "docs/benchmarks/transactional-rg-records-20261107.json.gz" + } } diff --git a/xff/engine/evaluate.cc b/xff/engine/evaluate.cc index 8025d8674e..11fbc9bc9e 100644 --- a/xff/engine/evaluate.cc +++ b/xff/engine/evaluate.cc @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -2172,22 +2173,30 @@ void EmitSelectedGrepLine( // Source bytes are consumed incrementally. Buffer rendered records until the complete scan // has validated binary detection and read errors; source and all-line vectors are never retained. -absl::StatusOr StreamGrepLines( +template +using GrepLineResult = std::conditional_t; + +template +absl::StatusOr> RenderGrepLines( const parser::Expr& expr, EvalContext& ctx, const GrepMatchers& matcher, - std::uint64_t sniff) { + std::uint64_t sniff, + std::string records = {}) { if (ctx.visit.metadata.type != vfs::FileType::kRegular) { - return false; + return GrepLineResult{}; } const bool only_matching = ctx.grep.only_matching && !(ctx.grep.rg_mode && ctx.grep.invert); const bool with_context = !only_matching && (ctx.grep_before != 0 || ctx.grep_after != 0); content::LineContext selection(with_context ? ctx.grep_before : 0, with_context ? ctx.grep_after : 0); - std::string rendered; - std::vector record_ends; + std::string rendered = std::move(records); + rendered.clear(); + std::conditional_t, const std::vector> record_ends; const auto buffer = [&](std::string_view text) { rendered.append(text); - record_ends.push_back(rendered.size()); + if constexpr (kPublishRecords) { + record_ends.push_back(rendered.size()); + } }; const auto sink = ctx.emit; bool first = true; @@ -2205,14 +2214,14 @@ absl::StatusOr StreamGrepLines( }; const auto visit = [&](std::size_t number, std::string_view text) { const bool selected = matcher.PartialMatch(text) != ctx.grep.invert; - matched = matched || selected; + matched |= selected; selection.Push(number, text, selected, emit_line); return true; }; if (ctx.content.Loaded()) { MBO_ASSIGN_OR_RETURN(const auto text, ctx.content.Read(ctx.fs, ctx.visit.path)); if (text.substr(0, static_cast(std::min(text.size(), sniff))).contains('\0')) { - return false; + return GrepLineResult{}; } content::VisitLines(text, visit); } else { @@ -2220,15 +2229,22 @@ absl::StatusOr StreamGrepLines( MBO_ASSIGN_OR_RETURN(const auto stream, source->Open()); MBO_ASSIGN_OR_RETURN(const auto scan, content::ScanLines(*stream, sniff, visit)); if (scan.binary) { - return false; + return GrepLineResult{}; } } - std::size_t start = 0; - for (const std::size_t end : record_ends) { - sink(std::string_view(rendered).substr(start, end - start)); - start = end; + if constexpr (kPublishRecords) { + std::size_t start = 0; + for (const std::size_t end : record_ends) { + sink(std::string_view(rendered).substr(start, end - start)); + start = end; + } + return matched; + } else { + return RenderedRgOutput{ + .selected = matched, + .text = std::move(rendered), + }; } - return matched; } bool EvalGrepMatchers(const parser::Expr& expr, EvalContext& ctx, const GrepMatchers& matcher) { @@ -2239,7 +2255,7 @@ bool EvalGrepMatchers(const parser::Expr& expr, EvalContext& ctx, const GrepMatc const auto summary = SummarizeGrep(ctx, matcher, content::kBinaryNulSniffBytes); return summary.ok() && EmitGrepSummary(expr, ctx, *summary); } - return StreamGrepLines(expr, ctx, matcher, content::kBinaryNulSniffBytes).value_or(false); + return RenderGrepLines(expr, ctx, matcher, content::kBinaryNulSniffBytes).value_or(false); } bool EvalGrep(const parser::Expr& expr, EvalContext& ctx) { @@ -4783,47 +4799,71 @@ absl::StatusOr EmitMultilineRg(const MatchOutput& output, const parser::Rg return selected_lines != 0; } -absl::StatusOr EmitRgRecords(const MatchOutput& output, const parser::RgSearch& search, EvalContext& context) { +absl::StatusOr RenderRgRecords( + const MatchOutput& output, + const parser::RgSearch& search, + EvalContext& context, + std::string records) { if (context.visit.metadata.type != vfs::FileType::kRegular) { - return false; + return RenderedRgOutput{}; } - if (search.multiline) { - return EmitMultilineRg(output, search, context); + if (!search.multiline && !ReducedGrepOutput(context)) { + const GrepMatchers matcher(output.matchers, search); + return RenderGrepLines( + *output.source, context, matcher, search.text ? 0 : std::numeric_limits::max(), + std::move(records)); } - const GrepMatchers matcher(output.matchers, search); - if (ReducedGrepOutput(context)) { + records.clear(); + const auto sink = context.emit; + const auto buffer = [&](std::string_view text) { records.append(text); }; + context.emit = buffer; + const auto result = [&] -> absl::StatusOr { + if (search.multiline) { + return EmitMultilineRg(output, search, context); + } + const GrepMatchers matcher(output.matchers, search); MBO_ASSIGN_OR_RETURN( const auto summary, SummarizeGrep(context, matcher, search.text ? 0 : std::numeric_limits::max())); return EmitGrepSummary(*output.source, context, summary); - } - return StreamGrepLines(*output.source, context, matcher, search.text ? 0 : std::numeric_limits::max()); + }(); + context.emit = sink; + MBO_ASSIGN_OR_RETURN(const bool selected, result); + return RenderedRgOutput{ + .selected = selected, + .text = std::move(records), + }; } } // namespace -absl::StatusOr EmitRgOutput(const MatchOutput& output, const parser::RgSearch& search, EvalContext& context) { - const auto sink = context.emit; +absl::StatusOr RenderRgOutput( + const MatchOutput& output, + const parser::RgSearch& search, + EvalContext& context, + std::string records) { const bool filename = context.grep.filename; const bool heading = context.grep.heading && filename && !context.grep_json && context.grep.output == GrepOptions::Output::kLines; - std::string records; - const auto buffer = [&](std::string_view text) { records.append(text); }; - context.emit = buffer; if (heading) { context.grep.filename = false; } - const auto result = EmitRgRecords(output, search, context); - context.emit = sink; + auto result = RenderRgRecords(output, search, context, std::move(records)); context.grep.filename = filename; - if (result.ok() && !records.empty()) { - if (heading) { - records.insert( - 0, absl::StrCat( - context.grep.color ? "\x1b[35m" : "", context.visit.path, context.grep.color ? "\x1b[0m" : "", "\n")); - } - sink(records); + MBO_ASSIGN_OR_RETURN(auto rendered, std::move(result)); + if (heading && !rendered.text.empty()) { + rendered.text.insert( + 0, absl::StrCat( + context.grep.color ? "\x1b[35m" : "", context.visit.path, context.grep.color ? "\x1b[0m" : "", "\n")); } - return result; + return rendered; +} + +absl::StatusOr EmitRgOutput(const MatchOutput& output, const parser::RgSearch& search, EvalContext& context) { + MBO_ASSIGN_OR_RETURN(const auto rendered, RenderRgOutput(output, search, context)); + if (!rendered.text.empty()) { + context.emit(rendered.text); + } + return rendered.selected; } bool EmitMatchOutput(const MatchOutput& output, EvalContext& context) { diff --git a/xff/engine/evaluate.h b/xff/engine/evaluate.h index 82baa6d6cf..8e1b6b1c7e 100644 --- a/xff/engine/evaluate.h +++ b/xff/engine/evaluate.h @@ -465,6 +465,20 @@ absl::StatusOr PrepareRgOutput( const vfs::FileSystem& fs, bool fold_case, bool smart_case); + +struct RenderedRgOutput { + bool selected = false; + std::string text; +}; + +// Own the transaction until the complete read/binary checks succeed. The caller +// may lend an old record's allocation by value; no bytes reach context.emit. +absl::StatusOr RenderRgOutput( + const MatchOutput& output, + const parser::RgSearch& search, + EvalContext& context, + std::string records = {}); + absl::StatusOr EmitRgOutput(const MatchOutput& output, const parser::RgSearch& search, EvalContext& context); // Validates the --diff-ignore token list and the --diff-ignore-matching regex, returning the diff --git a/xff/engine/expression_allocations_test.cc b/xff/engine/expression_allocations_test.cc index 3ffade081c..63619a4ae1 100644 --- a/xff/engine/expression_allocations_test.cc +++ b/xff/engine/expression_allocations_test.cc @@ -216,10 +216,12 @@ class DirectoryAllocationFs final : public AllocationFs { class OutputAllocationFs final : public AllocationFs { public: + explicit OutputAllocationFs(std::size_t padding = 1'024) : text_(std::string(padding, 'x') + "hit\n") {} + absl::StatusOr ReadContent(std::string_view) const override { return text_; } private: - const std::string text_ = std::string(1'024, 'x') + "hit\n"; + const std::string text_; }; struct AllocationTest : ::testing::Test { @@ -428,6 +430,56 @@ TEST_F(AllocationTest, RecordRepeatedMatcherOutputAllocationTraffic) { // Inputs, warmup, assertions and source-fixture construction are outside recording. } +TEST_F(AllocationTest, RecordRepeatedTransactionalRgOutputAllocationTraffic) { + for (const std::size_t padding : {1'024UZ, 131'072UZ}) { + const OutputAllocationFs fs(padding); + const vfs::Metadata metadata{.type = vfs::FileType::kRegular}; + const auto entries = [&] { + std::vector result; + result.reserve(64); + for (std::size_t index = 0; index < 64; ++index) { + result.emplace_back(Visit{.path = "root/a", .name = "a", .root = "root", .metadata = metadata, .fs = fs}); + } + return result; + }; + MBO_ASSERT_OK_AND_ASSIGN(const auto command, parser::Parse({"--rg", "hit", "root"})); + MBO_ASSERT_OK_AND_ASSIGN(const auto prepared, PrepareRgOutput(command, fs, false, false)); + const auto search = command.rg.value_or(parser::RgSearch{}); + ParallelMatch matcher( + {}, 1, false, + ParallelContentOutput{ + .output = prepared, + .rg = search, + .grep = {.line_number = false, .filename = false}, + }); + const auto& warm = matcher.Match(entries()); + EXPECT_THAT(warm, SizeIs(64)); + std::size_t capacity = 0; + for (const auto& result : warm) { + if (result.content) { + capacity += result.content->text.capacity(); + } + } + RecordProperty(absl::StrCat("rg_output_", padding, "_capacity"), std::to_string(capacity)); + for (const int repetition : {1, 2}) { + auto input = entries(); + const auto measured = MeasureAllocations([&] { + std::size_t bytes = 0; + for (const auto& result : matcher.Match(std::move(input))) { + if (result.content) { + bytes += result.content->text.size(); + } + } + return bytes; + }); + EXPECT_THAT(measured.value, Eq(64 * (padding + 4))); + RecordCounts(absl::StrCat("rg_output_", padding, "_", repetition), measured.counts); + } + } + // One inline worker keeps this isolated allocation diagnostic on the recorder thread. + // It exercises the production handoff; sanitizer duration is not performance evidence. +} + TEST_F(AllocationTest, ReusedFuzzyWorkspaceAvoidsHotScoreAllocations) { const fuzzy::FzfQuery query("fl"); fuzzy::ScoreWorkspace workspace; diff --git a/xff/engine/parallel_match.cc b/xff/engine/parallel_match.cc index 69775c0818..0397629d85 100644 --- a/xff/engine/parallel_match.cc +++ b/xff/engine/parallel_match.cc @@ -207,7 +207,14 @@ void ParallelMatch::EvaluateEntry( context.grep.filename = output_->root_count != 1 || visit.depth != 0; } if (output_->rg) { - content.selected = EmitRgOutput(prepared, *output_->rg, context); + auto rendered = RenderRgOutput(prepared, *output_->rg, context, std::move(content.text)); + if (rendered.ok()) { + content.selected = rendered->selected; + content.text = std::move(rendered->text); + } else { + content.selected = rendered.status(); + content.text.clear(); + } } else { EmitMatchOutput(prepared, context); } diff --git a/xff/engine/rg_test.cc b/xff/engine/rg_test.cc index fb63ad2cbc..b2e5bf0ae6 100644 --- a/xff/engine/rg_test.cc +++ b/xff/engine/rg_test.cc @@ -51,6 +51,7 @@ using ::mbo::testing::WithDropIndent; using ::testing::AllOf; using ::testing::Contains; using ::testing::ElementsAre; +using ::testing::ElementsAreArray; using ::testing::Eq; using ::testing::Field; using ::testing::Ge; @@ -377,6 +378,202 @@ TEST_F(RgEngineTest, ReusedRgResultSlotsClearSelectionErrorsAndOldOutput) { } } +TEST_F(RgEngineTest, NativeLineTransactionsKeepCallbackBoundariesAndContextSeparators) { + MBO_ASSERT_OK_AND_ASSIGN(const auto command, parser::Parse({"tree", "-content", "hit"})); + MBO_ASSERT_OK_AND_ASSIGN(const auto prepared, PrepareMatchOutput(*command.expression)); + fs.files.at("tree/a") = "before\nhit\nafter\nskip\nskip\nhit\nlast\n"; + const auto entries = RepeatedEntries(1); + std::vector records; + const auto sink = [&](std::string_view text) { records.emplace_back(text); }; + Control control; + EvalContext context{ + .visit = entries.front().AsVisit(), + .emit = sink, + .fs = fs, + .grep = {.line_number = false, .filename = false}, + .grep_before = 1, + .grep_after = 1, + .control = control, + }; + EXPECT_THAT(EmitMatchOutput(prepared, context), IsTrue()); + EXPECT_THAT( + records, ElementsAreArray({ + EqualsText("before\n"), + EqualsText("hit\n"), + EqualsText("after\n"), + EqualsText("--\n"), + EqualsText("skip\n"), + EqualsText("hit\n"), + EqualsText("last\n"), + })); +} + +TEST_F(RgEngineTest, NativeLineTransactionsPublishNothingAfterLateFailure) { + MBO_ASSERT_OK_AND_ASSIGN(const auto command, parser::Parse({"tree", "-content", "hit"})); + MBO_ASSERT_OK_AND_ASSIGN(const auto prepared, PrepareMatchOutput(*command.expression)); + const auto entries = RepeatedEntries(1); + fs.streaming = true; + fs.source_override = std::make_shared(false); + std::vector records; + const auto sink = [&](std::string_view text) { records.emplace_back(text); }; + Control control; + EvalContext context{ + .visit = entries.front().AsVisit(), + .emit = sink, + .fs = fs, + .grep = {.line_number = false, .filename = false}, + .control = control, + }; + EXPECT_THAT(EmitMatchOutput(prepared, context), IsFalse()); + EXPECT_THAT(records, IsEmpty()); + context.emit("restored\n"); + EXPECT_THAT(records, ElementsAre(EqualsText("restored\n"))); +} + +TEST_F(RgEngineTest, NativeLineTransactionsKeepTheirSinkWhenCallbackReplacesIt) { + MBO_ASSERT_OK_AND_ASSIGN(const auto command, parser::Parse({"tree", "-content", "hit"})); + MBO_ASSERT_OK_AND_ASSIGN(const auto prepared, PrepareMatchOutput(*command.expression)); + fs.files.at("tree/a") = "hit\nhit\n"; + const auto entries = RepeatedEntries(1); + std::vector records; + std::vector alternate_records; + const auto alternate = [&](std::string_view text) { alternate_records.emplace_back(text); }; + std::function replace_sink; + const auto sink = [&](std::string_view text) { + records.emplace_back(text); + replace_sink(); + }; + Control control; + EvalContext context{ + .visit = entries.front().AsVisit(), + .emit = sink, + .fs = fs, + .grep = {.line_number = false, .filename = false}, + .control = control, + }; + replace_sink = [&] { context.emit = alternate; }; + EXPECT_THAT(EmitMatchOutput(prepared, context), IsTrue()); + EXPECT_THAT(records, ElementsAre(EqualsText("hit\n"), EqualsText("hit\n"))); + EXPECT_THAT(alternate_records, IsEmpty()); + context.emit("tail\n"); + EXPECT_THAT(alternate_records, ElementsAre(EqualsText("tail\n"))); +} + +TEST_F(RgEngineTest, OwnedRgRecordsRetainScratchAndRestoreHeadingAndSink) { + MBO_ASSERT_OK_AND_ASSIGN(const auto command, parser::Parse({"--rg", "hit", "tree"})); + MBO_ASSERT_OK_AND_ASSIGN(const auto prepared, PrepareRgOutput(command, fs, false, false)); + const auto search = command.rg.value_or(parser::RgSearch{}); + const auto entries = RepeatedEntries(1); + Control control; + const auto sink = [&](std::string_view text) { output.append(text); }; + EvalContext context{ + .visit = entries.front().AsVisit(), + .emit = sink, + .fs = fs, + .grep = {.heading = true, .line_number = false, .filename = true}, + .control = control, + }; + std::string scratch(1'024, 'x'); + const auto capacity = scratch.capacity(); + MBO_ASSERT_OK_AND_ASSIGN(const auto rendered, RenderRgOutput(prepared, search, context, std::move(scratch))); + EXPECT_THAT(rendered.selected, IsTrue()); + EXPECT_THAT(rendered.text, EqualsText("tree/a\nhit\n")); + EXPECT_THAT(rendered.text, Property(&std::string::capacity, Ge(capacity))); + EXPECT_THAT(context.grep.filename, IsTrue()); + EXPECT_THAT(output, IsEmpty()); + context.emit("restored\n"); + EXPECT_THAT(output, EqualsText("restored\n")); +} + +TEST_F(RgEngineTest, OwnedRgRecordsDiscardLateFailureAndRestoreTheSink) { + MBO_ASSERT_OK_AND_ASSIGN(const auto command, parser::Parse({"--rg", "hit", "tree"})); + MBO_ASSERT_OK_AND_ASSIGN(const auto prepared, PrepareRgOutput(command, fs, false, false)); + const auto search = command.rg.value_or(parser::RgSearch{}); + const auto entries = RepeatedEntries(1); + fs.streaming = true; + fs.source_override = std::make_shared(false); + Control control; + const auto sink = [&](std::string_view text) { output.append(text); }; + EvalContext context{ + .visit = entries.front().AsVisit(), + .emit = sink, + .fs = fs, + .grep = {.heading = true, .line_number = false, .filename = true}, + .control = control, + }; + EXPECT_THAT( + RenderRgOutput(prepared, search, context, "stale"), + StatusIs(absl::StatusCode::kDataLoss, HasSubstr("late read failure"))); + EXPECT_THAT(output, IsEmpty()); + EXPECT_THAT(context.grep.filename, IsTrue()); + fs.source_override.reset(); + MBO_ASSERT_OK_AND_ASSIGN(const auto rendered, RenderRgOutput(prepared, search, context)); + EXPECT_THAT(rendered.text, EqualsText("tree/a\nhit\n")); + EXPECT_THAT(output, IsEmpty()); + EXPECT_THAT(EmitRgOutput(prepared, search, context), IsOkAndHolds(IsTrue())); + EXPECT_THAT(output, EqualsText("tree/a\nhit\n")); +} + +TEST_F(RgEngineTest, OwnedRgRecordsDiscardBinaryAndNonmatchingScratch) { + MBO_ASSERT_OK_AND_ASSIGN(const auto command, parser::Parse({"--rg", "hit", "tree"})); + MBO_ASSERT_OK_AND_ASSIGN(const auto prepared, PrepareRgOutput(command, fs, false, false)); + const auto search = command.rg.value_or(parser::RgSearch{}); + const auto entries = RepeatedEntries(1); + fs.streaming = true; + fs.files.at("tree/a") = "hit\n" + std::string(70'000, 'x') + '\0'; + Control control; + const auto sink = [&](std::string_view text) { output.append(text); }; + EvalContext context{ + .visit = entries.front().AsVisit(), + .emit = sink, + .fs = fs, + .grep = {.line_number = false, .filename = false}, + .control = control, + }; + MBO_ASSERT_OK_AND_ASSIGN(const auto binary, RenderRgOutput(prepared, search, context, "stale")); + EXPECT_THAT(binary.selected, IsFalse()); + EXPECT_THAT(binary.text, IsEmpty()); + fs.files.at("tree/a") = "miss\n"; + EXPECT_THAT( + RenderRgOutput(prepared, search, context, "old output"), + IsOkAndHolds(AllOf( + Field("selected", &RenderedRgOutput::selected, IsFalse()), + Field("text", &RenderedRgOutput::text, IsEmpty())))); + EXPECT_THAT(output, IsEmpty()); +} + +TEST_F(RgEngineTest, RgResultBuffersReuseTransferredCapacityWithinExistingBounds) { + MBO_ASSERT_OK_AND_ASSIGN(const auto command, parser::Parse({"--rg", "hit", "tree"})); + MBO_ASSERT_OK_AND_ASSIGN(const auto prepared, PrepareRgOutput(command, fs, false, false)); + const auto search = command.rg.value_or(parser::RgSearch{}); + ParallelMatch matcher( + {}, 4, false, + ParallelContentOutput{ + .output = prepared, + .rg = search, + .grep = {.match_output = true, .line_number = false, .filename = false}, + }); + fs.files.at("tree/a") = std::string(1'024, 'x') + "hit\n"; + EXPECT_THAT(matcher.Match(RepeatedEntries(64)), SizeIs(64)); + fs.files.at("tree/a") = "hit\n"; + for (const auto& result : matcher.Match(RepeatedEntries(64))) { + EXPECT_THAT( + result.content, + Optional(Field( + "text", &ContentResult::text, AllOf(EqualsText("hit\n"), Property(&std::string::capacity, Ge(1'024)))))); + } + fs.files.at("tree/a") = std::string(2 * ParallelMatch::kReusableRecordBytes, 'x') + "hit\n"; + EXPECT_THAT(matcher.Match(RepeatedEntries(64)), SizeIs(64)); + fs.files.at("tree/a") = "hit\n"; + for (const auto& result : matcher.Match(RepeatedEntries(1))) { + EXPECT_THAT( + result.content, + Optional(Field( + "text", &ContentResult::text, + AllOf(EqualsText("hit\n"), Property(&std::string::capacity, Le(ParallelMatch::kReusableRecordBytes)))))); + } +} + TEST_F(RgEngineTest, NativeParallelBatchesReuseReadsWithoutSharingEntryContent) { fs.files.clear(); for (std::size_t index = 0; index < 300; ++index) {