diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 175473f4080..008a122d203 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)-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)-20261102)\.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 a1e64f80677..aaef0369b59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 0.11.0 +- Aggregate ordinary comparison side totals without retaining duplicate per-path summary tables; category and effect-sensitive summaries keep their existing behavior. + - Prepare explicit hash-primary algorithms and encodings once while preserving per-context defaults and conditional expected-hash templates. - Prepare file-diff normalization once per action style during a run instead of recompiling ignored-line regexes for every file pair. diff --git a/TODO.md b/TODO.md index 577e04e3b95..f617ca5d61b 100644 --- a/TODO.md +++ b/TODO.md @@ -429,7 +429,14 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms Keep live direct-context defaults, expected-field observations and verification bookkeeping. Field-hash qualifier binding and broader template-family qualification remain under EP03/EP07 and O22; this does not mark those wider plans complete. -- [ ] O16-O22: Reconcile paired streaming compare, reusable stage pools, byte-bounded overlap, archive indexing/admission, +- [x] O16a: Qualify direct aggregation of ordinary comparison side totals against native/in-memory + output, memory and unchanged category/action controls on Linux and macOS. + Keep [the narrower retention change](docs/benchmarks/aggregate-comparison-totals-20261102.md) + separate from actual completed-frontier streaming; both full tree maps remain. +- [ ] O16b: Add and qualify completed-directory/frontier traversal before enabling bounded paired + comparison. Preserve empty/ignored completion, ordering, ignore inheritance, errors, effects, + archives and deferred selections; retain explicit fallbacks and avoid repeated shallow scans. +- [ ] O17-O22: Reconcile reusable stage pools, byte-bounded overlap, archive indexing/admission, optional bulk metadata, similarity representations and output churn with EP/JC/P acceptance. Keep macrobenchmark, kernel and diagnostic contracts separate. diff --git a/docs/benchmarks/aggregate-comparison-totals-20261102.json.gz b/docs/benchmarks/aggregate-comparison-totals-20261102.json.gz new file mode 100644 index 00000000000..b732d46931a Binary files /dev/null and b/docs/benchmarks/aggregate-comparison-totals-20261102.json.gz differ diff --git a/docs/benchmarks/aggregate-comparison-totals-20261102.md b/docs/benchmarks/aggregate-comparison-totals-20261102.md new file mode 100644 index 00000000000..af06629df64 --- /dev/null +++ b/docs/benchmarks/aggregate-comparison-totals-20261102.md @@ -0,0 +1,75 @@ +# Aggregate ordinary comparison totals (2026-11-02) + +## Decision + +Retain direct side aggregation for ordinary overall/type/extension comparison summaries when their scopes need only side totals, combined totals or roots. Previously even default left/right totals retained a summary vector and maps per path, plus a category map, then rebuilt those totals after pairing. The selected path keeps existing root aggregates and constructs the side columns directly. + +Eligibility uses the existing independent-filter audit. Any category scope, complex summary, action, traversal control, collection or verification retains the previous per-entry implementation. It adds no flag-name checks or new effect vocabulary. This qualification covers O16a only: the two ordered tree maps, comparison ordering, worker policy, reads and first-output latency are unchanged. O16b completed-directory streaming and O17/O18 stage scheduling remain open. + +## Measurement contracts + +Measured parent `4e77548bebf05152bb55b64dadd8e7ec9da8f539` (#1056), clean `clang_release`/ThinLTO variants changing only `run.cc`. Common benchmark/test sources are identical. Manifests retain compiler identity, build configuration, link actions and executable/source hashes. macOS uses warm APFS; Linux uses tmpfs and pins each case to its 1/3 CPU allowance. Native invocation includes process startup; in-memory whole-engine timing excludes parser/fixture construction. Neither is sanitizer timing. + +Per host: 72 native cases x three sessions (broad/deep trees, 10/1,000/10,000 paths, 1/3 requested workers, overall/type/extension totals and category/mixed/status controls), discarded warmups and fastest seven of nine rotating rounds. Complete stdout equality and independent physical side count/byte and comparison-category oracles must pass. Separate in-memory timing has 30 cases x three sessions x two variants. Confirmation adds a byte-identical baseline: eight native cases x five sessions and five in-memory cases x three sessions x three variants. Source/link/test logs and all slower controls are retained; no cross-tool normalization masks a within-host regression. + +## Native commands + +Median paired session change, negative is faster. + +| Fixture / task | Workers | Linux | macOS | +| :-------------------- | ------: | ------: | -----: | +| broad10 / overall | 1 | +0.02% | -0.38% | +| broad1000 / ext | 1 | -14.42% | -4.04% | +| broad10000 / ext | 1 | -16.80% | -4.95% | +| broad10000 / type | 3 | -22.26% | -5.96% | +| deep10000 / overall | 1 | -18.41% | -5.47% | +| deep10000 / type | 3 | -22.13% | -6.88% | +| broad10000 / category | 1 | +1.04% | +0.46% | +| deep10000 / mixed | 3 | -1.02% | +0.05% | +| broad10000 / status | 3 | +0.75% | +0.27% | + +## In-memory whole-engine timing + +Microseconds per run; median of each session's fastest-seven mean. Fixture construction and command parsing are outside the timed loop; run preflight, traversal, comparison, summaries and teardown remain inside. + +| Task / scope | Files | CPUs | Linux baseline us | Linux aggregate us | Linux delta | macOS baseline us | macOS aggregate us | macOS delta | +| :------------------------------------------------- | -----: | ---: | ----------------: | -----------------: | ----------: | ----------------: | -----------------: | ----------: | +| overall / compare | 10 | 1 | 44.44 | 41.94 | -5.62% | 57.81 | 54.17 | -6.30% | +| overall / compare | 10,000 | 1 | 9,884.16 | 5,069.58 | -48.71% | 9,699.73 | 3,683.39 | -62.03% | +| type / compare | 10,000 | 3 | 8,887.72 | 4,113.35 | -53.72% | 10,037.93 | 4,077.63 | -59.38% | +| ext / compare | 10,000 | 1 | 9,917.12 | 5,431.53 | -45.23% | 9,601.70 | 3,703.04 | -61.43% | +| overall / left-only,right-only,different,identical | 10,000 | 1 | 10,075.56 | 10,107.73 | +0.32% | 10,086.23 | 9,809.10 | -2.75% | +| ext / left-total,left-only | 10,000 | 3 | 8,824.97 | 8,699.36 | -1.42% | 10,082.80 | 9,879.85 | -2.01% | + +## Confirmation and slower controls + +An identical baseline participant makes scheduling/sample movement visible; no subtraction of that movement is applied to candidate results. + +| Fixture / task | CPUs | Initial Linux | Confirm Linux | Repeat Linux | Initial macOS | Confirm macOS | Repeat macOS | +| :-------------------- | ---: | ------------: | ------------: | -----------: | ------------: | ------------: | -----------: | +| broad10 / overall | 1 | +0.02% | +0.39% | +0.72% | -0.38% | +0.85% | +0.31% | +| deep10000 / overall | 1 | -18.41% | -18.20% | -0.30% | -5.47% | -4.78% | +0.22% | +| deep10000 / mixed | 3 | -1.02% | +0.37% | +1.99% | +0.05% | +1.16% | +0.63% | +| broad10000 / category | 1 | +1.04% | +1.33% | +1.28% | +0.46% | +1.23% | +1.01% | +| broad10 / status | 3 | +2.21% | -0.75% | -1.30% | -0.06% | +1.12% | +0.54% | +| deep10000 / type | 3 | -22.13% | -21.48% | -0.81% | -6.88% | -5.16% | +0.10% | +| broad10000 / status | 3 | +0.75% | -0.67% | +1.60% | +0.27% | +0.39% | +1.04% | +| broad10000 / ext | 1 | -16.80% | -15.59% | -0.66% | -4.95% | -4.59% | +0.36% | + +## Memory, size and correctness + +Peak RSS is from the native launcher and includes the complete process, not only summary storage. Whole-tree retention remains linear; this change does not introduce a bounded streaming algorithm. + +| Host / large overall fixture | Baseline peak MiB | Aggregate peak MiB | +| :--------------------------- | ----------------: | -----------------: | +| linux / broad10000 | 16.46 | 14.09 | +| macos / broad10000 | 18.03 | 14.53 | + +All 323 extension-enabled release tests pass on each host, with required Linux FUSE. Five focused ASan suites pass on each host; five Linux TSan suites pass. All three modified translation units pass scoped local lint. Dedicated read-only VFS tests compare direct totals against the existing action fallback across scopes, filters and repeated summaries; independent expected counts cover directory roots, file/directory changes and empty sides. Formatting oracles compare complete baseline/candidate output. Normal enforcing hooks and current pushed-head CI remain required. + +- linux: unstripped CLI 12,387,664 -> 12,393,880 bytes (+6,216). +- macos: unstripped CLI 4,078,832 -> 4,079,152 bytes (+320). + +## Lossless evidence + +[aggregate-comparison-totals-20261102.json.gz](aggregate-comparison-totals-20261102.json.gz): 1,217,750 bytes; SHA-256 `c5c9037feb003ea1a5bf5cdffb44d9baa561706ec7a9392f5d270d921211a14f`. diff --git a/docs/design-summary-scopes.md b/docs/design-summary-scopes.md index e4ff81cf4da..7e9b9fe3f79 100644 --- a/docs/design-summary-scopes.md +++ b/docs/design-summary-scopes.md @@ -74,7 +74,8 @@ xff --compare LEFT RIGHT --summary --summary-scope=diff Repeating `--summary` with another grouping produces another table. Accumulation retains the existing archive, collection, shard, and verification feeds. Combined -and root scopes aggregate during traversal. Category scopes retain per-entry contributions until +and root scopes aggregate during traversal. Ordinary overall/type/extension side totals also aggregate +directly for independently audited filters when no category columns are requested. Category scopes retain per-entry contributions until pairing assigns categories, without replaying expressions or executing actions again. Entries without a comparison category do not enter category tables, even if a special reduction feed (such as failed hash verification) counted them in a whole-root table. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index 0efebc8cbba..7ae0a5875d0 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -2822,10 +2822,10 @@ only on its first nonempty text copy. Growth-policy controls remain separate evi ## Reconciled cross-platform optimization audit after #1040 -Baseline: merged main `16e70cb97de93c7ab4ae8c9e990e1842d50cc528`. This is a code and evidence audit, -not a speedup claim for every candidate. Only the collection text-sharing experiment below has new -Linux/macOS timing evidence. Preserve the distinction between release timing, allocation accounting, -in-memory kernels, and correctness tests. +Initial audit baseline: merged main `16e70cb97de93c7ab4ae8c9e990e1842d50cc528`. The inventory is +not a speedup claim for every candidate. The decision sections below record subsequent independent +Linux/macOS qualification, including rejected candidates. Preserve the distinction between release +timing, allocation accounting, in-memory kernels, and correctness tests. ### Existing work and decisions @@ -3417,3 +3417,40 @@ are retained with compiler/link-action provenance. No toolchain/LTO change is pa This closes the hash-primary subset of O15. Field-hash qualifier preparation and digest/output-copy work remain associated with O22, and broader EP03/EP07 template-family qualification stays open. + +### O16a: aggregate ordinary comparison side totals + +Default `left-total`/`right-total` summaries previously retained per-path summary vectors and maps, +then reconstructed side totals through a second category map. Ordinary `overall`, `type` and `ext` +summary sinks can aggregate directly when no category scope is requested and the expression is absent +or passes the existing independent-filter audit. Category/mixed scopes, complex summaries and +expressions with actions, traversal controls, collection or verification keep the existing path. +No worker, lock, clock, flag or observation moves. + +[The qualification report](benchmarks/aggregate-comparison-totals-20261102.md) separates native +commands, in-memory whole-engine timing, memory, identical-baseline controls and footprint. This is +retention of duplicate summary storage only: both complete tree maps remain, and first-output latency +is unchanged. O16b, O17/O18 and the broader JC plans remain open. + +### O16b/O17/O18: completed-frontier integration remains open + +The current walker exposes visits, not completed-directory events or a resumable cursor. Absence +from a filtered matched-entry callback cannot prove directory completion. Its directory-block and +subtree traversal orders are not uniformly global lexicographic streams, so wrapping callbacks in +bounded queues cannot by itself classify unmatched entries or guarantee progress. + +`RunTreeCompare` also completes both walks before emitting comparison records and emits none if a +walk fails. Moving comparison ahead of later expressions can change the observed file bytes when +those expressions have effects. A streaming implementation must explicitly retain or qualify these +error/effect semantics, plus ignore inheritance, archive owners, depth/limits, deferred selections +and ordering. Repeated shallow `RunFindCore` calls are unsuitable: they repeat preparation and lose +root-relative traversal/ignore context. + +A preparatory interface needs coordinator-owned completed-frontier events, including positive +completion for empty, ignored and unmatched directories, after pending matching results publish. +Then matching frontiers can release retained entries when both sides prove that no counterpart can +arrive later. Qualify a serial cursor first; separately measure persistent role coordination and +byte-bounded queues. Directory jobs must remain leaf work and never wait for nested tasks on their +own saturated pool. O08 task storage and O11 warm participation do not establish that a shared +executor is faster. Global sorting/effects/deferred/archive cases need explicit fallback contracts +until independently qualified. diff --git a/release-site.json b/release-site.json index 6fedecd359a..fda6ae62d51 100644 --- a/release-site.json +++ b/release-site.json @@ -74,7 +74,8 @@ "docs/benchmarks/derived-content-handoff-20261102.md": "docs/benchmarks/derived-content-handoff-20261102.html", "docs/benchmarks/immutable-default-catalogs-20261102.md": "docs/benchmarks/immutable-default-catalogs-20261102.html", "docs/benchmarks/prepared-diff-configuration-20261102.md": "docs/benchmarks/prepared-diff-configuration-20261102.html", - "docs/benchmarks/prepared-hash-operands-20261102.md": "docs/benchmarks/prepared-hash-operands-20261102.html" + "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" }, "generated_html": { "XFF.md": { diff --git a/xff/engine/BUILD.bazel b/xff/engine/BUILD.bazel index e8eb07932a1..2f5d4508f63 100644 --- a/xff/engine/BUILD.bazel +++ b/xff/engine/BUILD.bazel @@ -1068,3 +1068,20 @@ cc_test( "@mboworks_mbo//mbo/testing:status_cc", ], ) + +cc_test( + name = "comparison_totals_test", + size = "small", + srcs = ["comparison_totals_test.cc"], + deps = [ + ":run_cc", + "//xff/parser:parser_cc", + "//xff/vfs:vfs_cc", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@googletest//:gtest_main", + "@mboworks_mbo//mbo/status:status_macros_cc", + "@mboworks_mbo//mbo/testing:status_cc", + "@nlohmann_json//:json", + ], +) diff --git a/xff/engine/compare_benchmark.cc b/xff/engine/compare_benchmark.cc index 46da5c0d04d..421dd6bcc02 100644 --- a/xff/engine/compare_benchmark.cc +++ b/xff/engine/compare_benchmark.cc @@ -126,6 +126,46 @@ void Compare(benchmark::State& state, std::size_t bytes, bool different, bool pa } state.SetItemsProcessed(state.iterations() * state.range(0)); } + +void CompareTotals(benchmark::State& state, std::string_view summary, std::string_view scopes) { + const CompareTree tree(static_cast(state.range(0)), 8, true); + const std::vector arguments{ + "--compare=summary", + "--summary=" + std::string(summary), + "--summary-scope=" + std::string(scopes), + "--jobs=" + std::to_string(state.range(1)), + "--archive=none", + "--sort=none", + "left", + "right", + "-type", + "f", + }; + auto command = xff::parser::Parse(arguments); + if (!command.ok()) { + state.SkipWithError(command.status().ToString()); + return; + } + std::size_t output_bytes = 0; + const auto emit = [&](std::string_view text) { output_bytes += text.size(); }; + const auto error = [](std::string_view, absl::Status) {}; + const auto initial = xff::engine::RunFind(*command, tree, emit, error); + if (initial.errors != 0 || output_bytes == 0) { + state.SkipWithError("incorrect comparison summary output"); + return; + } + for (auto iteration : state) { + benchmark::DoNotOptimize(iteration); + output_bytes = 0; + const auto result = xff::engine::RunFind(*command, tree, emit, error); + if (result.errors != 0) { + state.SkipWithError("comparison summary failed"); + break; + } + benchmark::DoNotOptimize(output_bytes); + } + state.SetItemsProcessed(state.iterations() * state.range(0)); +} } // namespace // XFF_ABI_POINTER: Google Benchmark's process argument interface. @@ -152,6 +192,27 @@ int main(int argc, char** argv) { ->UseRealTime() ->Unit(benchmark::kMicrosecond); } + + struct TotalsScenario { + std::string_view summary; + std::string_view scopes; + }; + + constexpr auto kTotals = std::to_array({ + {.summary = "overall", .scopes = "compare"}, + {.summary = "type", .scopes = "compare"}, + {.summary = "ext", .scopes = "compare"}, + {.summary = "overall", .scopes = "left-only,right-only,different,identical"}, + {.summary = "ext", .scopes = "left-total,left-only"}, + }); + for (const auto& scenario : kTotals) { + benchmark::RegisterBenchmark( + "totals/" + std::string(scenario.summary) + "/" + std::string(scenario.scopes), CompareTotals, scenario.summary, + scenario.scopes) + ->ArgsProduct({{10, 100, 1'000, 10'000}, {1, 3}}) + ->UseRealTime() + ->Unit(benchmark::kMicrosecond); + } benchmark::RunSpecifiedBenchmarks(); benchmark::Shutdown(); } diff --git a/xff/engine/comparison_totals_test.cc b/xff/engine/comparison_totals_test.cc new file mode 100644 index 00000000000..4ab1d94c77d --- /dev/null +++ b/xff/engine/comparison_totals_test.cc @@ -0,0 +1,259 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "mbo/status/status_macros.h" +#include "mbo/testing/status.h" +#include "nlohmann/json.hpp" +#include "xff/engine/run.h" +#include "xff/parser/parser.h" +#include "xff/vfs/entry.h" +#include "xff/vfs/filesystem.h" + +namespace xff::engine { +namespace { +using ::testing::Eq; +using ::testing::IsEmpty; +using ::testing::IsTrue; +using ::testing::Not; +using Json = nlohmann::json; + +struct ComparisonTotalsFs final : vfs::FileSystem { + const std::map files{ + {"left/a.txt", "x\n"}, {"left/b.cc", "before\n"}, {"left/l.txt", "left\n"}, {"left/sub/x.h", "two\n"}, + {"left/change/item", "l\n"}, {"right/a.txt", "x\n"}, {"right/b.cc", "after\n"}, {"right/r.md", "right\n"}, + {"right/sub/x.h", "two\n"}, {"right/change", "r\n"}, + }; + bool empty_left = false; + mutable std::atomic reads = 0; + // Comparison walks read concurrently; this counter measures calls only. + // The fixture data is immutable and never synchronized through this counter. + mutable std::atomic observations = 0; + + bool Directory(std::string_view path) const { + const std::string prefix = std::string(path) + "/"; + return std::ranges::any_of(files, [&](const auto& file) { return file.first.starts_with(prefix); }); + } + + absl::StatusOr> ReadDir(std::string_view path) const override { + ++observations; + if (path == "left" && empty_left) { + return std::vector{}; + } + if (!Directory(path)) { + return absl::NotFoundError("unknown fixture directory"); + } + const std::string prefix = std::string(path) + "/"; + std::map entries; + for (const auto& [name, content] : files) { + if (!name.starts_with(prefix)) { + continue; + } + const auto relative = std::string_view(name).substr(prefix.size()); + const auto slash = relative.find('/'); + const std::string child(relative.substr(0, slash)); + entries.try_emplace( + child, vfs::Entry{ + .path = prefix + child, + .name = child, + .type = slash == std::string_view::npos ? vfs::FileType::kRegular : vfs::FileType::kDirectory, + }); + } + std::vector result; + result.reserve(entries.size()); + for (auto& [name, entry] : entries) { + result.push_back(std::move(entry)); + } + return result; + } + + absl::StatusOr Stat(std::string_view path, bool) const override { + ++observations; + if (const auto file = files.find(std::string(path)); file != files.end()) { + return vfs::Metadata{ + .type = vfs::FileType::kRegular, + .size = file->second.size(), + .mode = 0644, + .ino = std::hash{}(path), + }; + } + if (Directory(path)) { + // Distinct ancestors must have distinct inode identities for loop detection. + return vfs::Metadata{.type = vfs::FileType::kDirectory, .ino = std::hash{}(path)}; + } + return absl::NotFoundError("unknown fixture entry"); + } + + absl::StatusOr ReadContent(std::string_view path) const override { + ++observations; + ++reads; + if (const auto file = files.find(std::string(path)); file != files.end()) { + return file->second; + } + return absl::NotFoundError("unknown fixture contents"); + } + + absl::Status Remove(std::string_view) const override { + ADD_FAILURE() << "Comparison summaries must not mutate their input"; + return absl::PermissionDeniedError("read-only fixture"); + } + + bool Access(std::string_view, vfs::AccessMode) const override { return true; } + + absl::StatusOr ReadLink(std::string_view) const override { return absl::NotFoundError("not a link"); } + + absl::StatusOr FsType(std::string_view) const override { return "memory"; } + + absl::StatusOr IsCaseSensitive(std::string_view) const override { return true; } +}; + +struct TotalsOutput { + RunResult result; + std::vector rows; + std::vector errors; + std::size_t reads = 0; +}; + +struct ComparisonTotalsTest : ::testing::Test { + absl::StatusOr Run( + std::vector options, + bool action = false, + const std::vector& expression = {}) { + options.insert(options.begin(), {"--compare=summary", "--format=jsonl"}); + options.insert(options.end(), {"left", "right"}); + options.insert(options.end(), expression.begin(), expression.end()); + if (action) { + options.push_back("-print"); + } + MBO_ASSIGN_OR_RETURN(const auto command, parser::Parse(options)); + TotalsOutput output; + filesystem.reads = 0; + const auto emit = [&](std::string_view text) { + const auto row = Json::parse(text, nullptr, false); + if (!row.is_discarded() && row.contains("left-total")) { + output.rows.push_back(row); + } + }; + output.result = RunFind(command, filesystem, emit, [&](std::string_view, absl::Status error) { + output.errors.push_back(std::move(error)); + }); + output.reads = filesystem.reads.load(); + return output; + } + + ComparisonTotalsFs filesystem; +}; + +TEST_F(ComparisonTotalsTest, OrdinaryTotalsMatchTheActionFallbackAcrossScopesAndSinks) { + constexpr auto kScopes = std::to_array({ + "compare", + "left-total,right-total", + "root,left-total,right-total,all", + "left-total,left-only", + }); + constexpr auto kSummaries = std::to_array({ + "overall", + "type", + "ext", + }); + for (const auto scope : kScopes) { + for (const auto summary : kSummaries) { + SCOPED_TRACE(scope); + SCOPED_TRACE(summary); + const std::vector arguments{ + "--summary-scope=" + std::string(scope), + "--summary=" + std::string(summary), + }; + ASSERT_OK_AND_ASSIGN(const auto ordinary, Run(arguments)); + ASSERT_OK_AND_ASSIGN(const auto action, Run(arguments, true)); + EXPECT_THAT(ordinary.result.errors, Eq(0)); + EXPECT_THAT(action.result.errors, Eq(0)); + EXPECT_THAT(ordinary.errors, IsEmpty()); + EXPECT_THAT(action.errors, IsEmpty()); + EXPECT_THAT(ordinary.rows, Not(IsEmpty())); + EXPECT_THAT(ordinary.rows, Eq(action.rows)); + EXPECT_THAT(ordinary.reads, Eq(action.reads)); + EXPECT_THAT(ordinary.result.any_match, IsTrue()); + } + } +} + +TEST_F(ComparisonTotalsTest, IndependentFiltersAndMultipleSummarySinksMatchTheFallback) { + const auto expressions = std::to_array>({ + {"-type", "f"}, + {"-name", "*.txt"}, + {"-name", "never-matches"}, + {"(", "-type", "d", "+", "-name", "*.h", ")"}, + }); + const std::vector arguments{ + "--summary=overall", + "--summary=type", + "--summary=ext", + "--summary-scope=compare", + }; + for (const auto& expression : expressions) { + ASSERT_OK_AND_ASSIGN(const auto ordinary, Run(arguments, false, expression)); + ASSERT_OK_AND_ASSIGN(const auto action, Run(arguments, true, expression)); + EXPECT_THAT(ordinary.result.errors, Eq(0)); + EXPECT_THAT(action.result.errors, Eq(0)); + EXPECT_THAT(ordinary.errors, IsEmpty()); + EXPECT_THAT(action.errors, IsEmpty()); + EXPECT_THAT(ordinary.rows, Not(IsEmpty())); + EXPECT_THAT(ordinary.rows, Eq(action.rows)); + EXPECT_THAT(ordinary.reads, Eq(action.reads)); + EXPECT_THAT(ordinary.result.any_match, Eq(action.result.any_match)); + } +} + +TEST_F(ComparisonTotalsTest, DirectoryRootsAndTypeTransitionsContributeOnceToEachTotal) { + ASSERT_OK_AND_ASSIGN(const auto result, Run({"--summary=overall"})); + EXPECT_THAT(result.result.errors, Eq(0)); + EXPECT_THAT(result.errors, IsEmpty()); + EXPECT_THAT(result.rows, Not(IsEmpty())); + bool found = false; + for (const auto& row : result.rows) { + if (!row.value("is_total", false)) { + continue; + } + found = true; + EXPECT_THAT(row.at("left-total").at("count").get(), Eq(8)); + EXPECT_THAT(row.at("right-total").at("count").get(), Eq(7)); + EXPECT_THAT(row.at("left-total").at("bytes").get(), Eq(20)); + EXPECT_THAT(row.at("right-total").at("bytes").get(), Eq(20)); + } + EXPECT_THAT(found, IsTrue()); +} + +TEST_F(ComparisonTotalsTest, EmptyPhysicalSideKeepsItsDirectoryRootContribution) { + filesystem.empty_left = true; + ASSERT_OK_AND_ASSIGN(const auto ordinary, Run({"--summary=overall"})); + ASSERT_OK_AND_ASSIGN(const auto action, Run({"--summary=overall"}, true)); + EXPECT_THAT(ordinary.result.errors, Eq(0)); + EXPECT_THAT(ordinary.rows, Eq(action.rows)); + bool found = false; + for (const auto& row : ordinary.rows) { + if (row.value("is_total", false)) { + found = true; + EXPECT_THAT(row.at("left-total").at("count").get(), Eq(1)); + EXPECT_THAT(row.at("right-total").at("count").get(), Eq(7)); + } + } + EXPECT_THAT(found, IsTrue()); +} +} // namespace +} // namespace xff::engine diff --git a/xff/engine/run.cc b/xff/engine/run.cc index cc2638ba790..a8c904c8123 100644 --- a/xff/engine/run.cc +++ b/xff/engine/run.cc @@ -4351,17 +4351,54 @@ SummaryTables PairSummaryContributions(const SummaryTables& left, const SummaryT return result; } +// Ordinary side totals do not consume pair categories. Keep the per-entry path for +// category scopes and for reductions/effects outside the existing independent-filter audit. +bool CanAggregateComparisonTotals( + const std::vector& scopes, + const std::vector& summaries, + mbo::types::OptionalRef expression) { + return absl::c_none_of( + scopes, [](std::string_view scope) { return absl::c_linear_search(kComparisonCategories, scope); }) + && absl::c_all_of( + summaries, + [](const SummarySpec& summary) { + return summary.mode == SummaryMode::kOverall || summary.mode == SummaryMode::kType + || summary.mode == SummaryMode::kExt; + }) + && (!expression || CanParallelMatch(*expression)); +} + +SummaryTables AggregatedComparisonColumn( + std::string_view scope, + const std::array& sides, + std::size_t sinks) { + if (scope == "left-total") { + return CombinedSummaryTables(sides.at(0)); + } + if (scope == "right-total") { + return CombinedSummaryTables(sides.at(1)); + } + return SummaryTables(sinks); +} + std::vector ComparisonScopeColumns( const std::vector& scopes, const std::array& sides, const std::map& categories, std::size_t sinks) { + const bool aggregated = !sides.at(0).per_entry && !sides.at(1).per_entry; std::vector columns; for (const auto& scope : scopes) { if (scope != "all" && scope != "root") { - columns.push_back({.scope = scope, .tables = SummaryTables(sinks)}); + columns.push_back({ + .scope = scope, + .tables = aggregated ? AggregatedComparisonColumn(scope, sides, sinks) : SummaryTables(sinks), + }); } } + if (aggregated) { + return columns; + } for (const auto& [path, category] : categories) { const auto left = ComparisonContribution(sides.at(0), path); const auto right = ComparisonContribution(sides.at(1), path); @@ -5267,7 +5304,8 @@ RunResult RunFindCore( .sinks = summaries.size(), .per_root = compare_listing || absl::c_linear_search(scopes, "root"), .per_entry = !summaries.empty() && compare_listing - && absl::c_any_of(scopes, [](std::string_view scope) { return scope != "all" && scope != "root"; }), + && absl::c_any_of(scopes, [](std::string_view scope) { return scope != "all" && scope != "root"; }) + && !CanAggregateComparisonTotals(scopes, summaries, expression), }; // --histogram (repeatable): a bar chart of the count per bucket, alongside or instead of // --summary. Both are reductions fed by one walk; a run with either suppresses the listing.