diff --git a/.bazelrc b/.bazelrc index 831d59903f8..a31edb97a5d 100644 --- a/.bazelrc +++ b/.bazelrc @@ -140,6 +140,11 @@ common:clang_release --config=clang --config=release common:linux_perf --define=pfm=1 common:linux_perf --downloader_config=bazelmod/linux_perf_downloader.cfg +# Unconfigured source queries visit every select arm, including Google Benchmark's +# optional libpfm dependency. Repair that one retired mirror for queries too, +# without enabling hardware counters or changing default/release build settings. +query --downloader_config=bazelmod/linux_perf_downloader.cfg + # --config=clang-coverage : collect C++ coverage with the same hermetic Clang/libc++ toolchain # that owns the default build. Bazel otherwise defaults C++ coverage to the local gcov toolchain. common:clang-coverage --config=clang diff --git a/TODO.md b/TODO.md index c42c3cd2ec4..7b55ef84f8b 100644 --- a/TODO.md +++ b/TODO.md @@ -144,12 +144,19 @@ scope or declaring new flags implemented. See useful task size, throughput, first output, memory and actual thread/child counts. Include serial command/concurrent-discovery, strict dependent expressions and parallel command controls. Do not accept a default or performance claim from hosted correctness checks alone. -- [ ] JC07: Separate directory-admission tuning from worker-budget tuning. PR #991's experimental - 64-sibling gate is not an optimum across storage types. Design explicit serial operation and - minimum independent-work/cost controls in the jobs family, preserving semantic/safety barriers - and command resource bounds. Distinguish cold start from warm dispatch, and qualify settings - on small slow frontiers as well as broad memory-backed workloads before choosing defaults. - Keep spellings unsettled until designed; follow JC01's unstable gate and self-documentation. +- [ ] JC07: Separate directory-admission tuning from worker-budget tuning. After #1034 removed + Abseil clock calibration, #991's 4 ms saved-work gate and synchronous first-child sample + are not recommended: Linux cheap scans save about 0.21 ms, while slow-child controls regress + by 28-59%; macOS broad/nested 4,096-file scans regress by 7-9%. Current paired measurements, + raw evidence and the retention recommendation are in docs/performance-analysis.md. Keep + #991 draft; preserve its independent libpfm query-downloader repair if admission is discarded. + Investigate actual pool startup/queue cost and bounded run-owned worker reuse without + serializing small expensive frontiers. Qualify heterogeneous reads and real slow storage. + Design explicit serial operation and minimum independent-work/cost controls in the jobs + family, preserving semantic/safety barriers and command resource bounds. Distinguish cold + start from warm dispatch; qualify settings on small slow frontiers and broad memory-backed + workloads before choosing defaults. Keep spellings unsettled until designed; follow JC01's + unstable gate and self-documentation. - [ ] JC08: Add opt-in, super-low-overhead scheduling analysis for admission tuning. Select diagnostics once at command setup; keep disabled inner loops free of diagnostic checks, timers, counters, atomics and allocations. Use bounded per-role/worker aggregates from existing observations: diff --git a/XFF.md b/XFF.md index ebb17de03ce..f3ba16ae762 100644 --- a/XFF.md +++ b/XFF.md @@ -565,7 +565,7 @@ See also: [--help=config](#topic-config), [--help=archive](#topic-archive), [--h - `-j N, -j=N, --jobs=N|all` - directory-read, eligible content-match, comparison and -exec workers _(global, find, xff, rg)_ - `N` is a positive integer; use `-j 4`, `-j=4`, or `--jobs=4`. The conventional attached short form `-j4` is also accepted. Every form accepts `all` in place of `N`. `-j 1` makes directory reads and `-exec ... ;` synchronous. At larger values, xff reads directories on up to `N` worker threads; large directory listings can split required eager metadata reads across the same pool. Lazy and metadata-free listings do not start stat workers. Independent content predicates and rg searches also match in bounded parallel batches, with output kept in traversal order. Small initial batches, archive members, stateful expressions stay on the coordinator. Metadata-consuming output does not itself prevent eligible content filters from running in workers; reductions and rendering remain serial. Independent regular-file comparison pairs can also use bounded workers, preserving relative-path output order. Batches of only small or metadata-only comparisons stay inline; archive and owned filesystem sources stay serial. Xff may independently keep up to `N` semicolon-form `-exec` / `-execdir` children outstanding. Reads and children can overlap; `N` is not one shared operation budget. The children's truth value is therefore success on launch. The `... +` batch forms still run once after the walk and propagate a failing exit status. With no flag, xff uses one fewer than the detected cores, capped at 15 and floored at 1; find and rg modes use every detected core. `all` always means every detected core. + `N` is a positive integer; use `-j 4`, `-j=4`, or `--jobs=4`. The conventional attached short form `-j4` is also accepted. Every form accepts `all` in place of `N`. `-j 1` makes directory reads and `-exec ... ;` synchronous. At larger values, xff reads directories on up to `N` worker threads; large directory listings can split required eager metadata reads across the same pool. Directory startup uses observed read cost and remaining independent work; small cheap batches can stay inline, while an existing pool is reused. `N` is a limit, not a promise to parallelize every listing. Lazy and metadata-free listings do not start stat workers. Independent content predicates and rg searches also match in bounded parallel batches, with output kept in traversal order. Small initial batches, archive members, stateful expressions stay on the coordinator. Metadata-consuming output does not itself prevent eligible content filters from running in workers; reductions and rendering remain serial. Independent regular-file comparison pairs can also use bounded workers, preserving relative-path output order. Batches of only small or metadata-only comparisons stay inline; archive and owned filesystem sources stay serial. Xff may independently keep up to `N` semicolon-form `-exec` / `-execdir` children outstanding. Reads and children can overlap; `N` is not one shared operation budget. The children's truth value is therefore success on launch. The `... +` batch forms still run once after the walk and propagate a failing exit status. With no flag, xff uses one fewer than the detected cores, capped at 15 and floored at 1; find and rg modes use every detected core. `all` always means every detected core. See also: [--help=output](#topic-output), [--help=stats](#topic-stats) diff --git a/docs/benchmarks/directory-admission-20261031.json.gz b/docs/benchmarks/directory-admission-20261031.json.gz new file mode 100644 index 00000000000..f026970a3b9 Binary files /dev/null and b/docs/benchmarks/directory-admission-20261031.json.gz differ diff --git a/docs/benchmarks/directory-admission-post-clock-20261101.json.gz b/docs/benchmarks/directory-admission-post-clock-20261101.json.gz new file mode 100644 index 00000000000..f8de690f490 Binary files /dev/null and b/docs/benchmarks/directory-admission-post-clock-20261101.json.gz differ diff --git a/docs/design-parallel.md b/docs/design-parallel.md index f3d90af2668..2206f6e5eea 100644 --- a/docs/design-parallel.md +++ b/docs/design-parallel.md @@ -44,8 +44,14 @@ A bounded directory-read worker pool with a single coordinator: - Workers perform only `ReadDir` plus metadata lookup and return complete listings. The coordinator submits sibling-directory reads ahead, then consumes their - futures in the order required by `--sort`. Directory workers start only when at least - two sibling directories provide independent work. For a coordinator-read directory with at least + futures in the order required by `--sort`. At least two sibling directories must provide + independent work. Cold startup additionally uses the last coordinator read's cost and the + estimated number of parallel rounds saved. If the parent does not justify startup, one child + is sampled inline and its ready result is retained, so it is never read twice. Cheap work stays + inline; expensive small batches can activate workers. An existing pool bypasses cold admission. + Only the coordinator records timing, only while the pool is cold; workers never share mutable + timing state. The admission budget is a heuristic, not a latency guarantee or an optimal policy + for every filesystem. For a coordinator-read directory with at least 512 children and eagerly required metadata, 128-entry stat chunks use the same pool. At most one chunk per worker is pending; worker jobs never wait on nested jobs. Metadata-free, lazy and owned/archive-source listings keep their serial per-directory path. diff --git a/docs/native-profiling.md b/docs/native-profiling.md index e320bccd348..ec3da5d13ef 100644 --- a/docs/native-profiling.md +++ b/docs/native-profiling.md @@ -28,7 +28,7 @@ filter, or timing columns alone do not establish working counters. Use The short duration above is a capability probe, not performance evidence. Google Benchmark 1.9.5 pins libpfm 4.11.0.bcr.1. That BCR source names the retired -`netcologne.dl.sourceforge.net` mirror. The opt-in downloader configuration +`netcologne.dl.sourceforge.net` mirror. The downloader configuration redirects exactly that archive to `downloads.sourceforge.net`, preserving the pinned version, BCR patches and integrity check. No LLVM/toolchain dependency changes are involved. It does not rewrite other archives or disable checksum @@ -36,6 +36,13 @@ validation. If network access is unavailable, an independently verified copy of `libpfm-4.11.0.tar.gz` can be supplied through Bazel's `--distdir=PATH`; do not substitute an unverified system library. +Unconfigured `bazel query` traverses all `select` arms, including the optional +libpfm dependency, even when hardware counters are disabled. Queries therefore +use the same narrowly scoped download repair so compile-source indexing can +complete. This does not enable counters or change default/release builds, +compiler/linker selection or macOS runtime behavior. Source indexing still +fails closed on any unsuccessful query. + ## Kernel and external perf capability The `perf` executable is a separate host profiling tool, not a libpfm dependency. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index b0197a4f552..b4c002dd09c 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -2610,6 +2610,43 @@ both native performance and the extra preparation/storage cost before the retent The capacity metric conservatively budgets inline strings and shared template transforms; it excludes backend, status and allocator bookkeeping. It must not be substituted for native allocation profiling. +## Directory read admission: revised #991 (2026-10-31) + +Tested current main `3803544ef4a24e0a72e54a317ee5091638bc70ae` against the original 64-sibling gate and a cost-aware replacement on drogon (Linux/KVM, AMD Ryzen 9 9950X, ten vCPUs). The replacement separates cold startup from reuse. It estimates saved read rounds from the last coordinator read; when that is insufficient, it samples one child and retains its ready result. An existing pool bypasses cold admission. Only the coordinator measures reads, while the pool is cold; there is no per-entry timing, worker-side timing state or new timing lock. + +The startup budget is 4 ms of estimated saved serial work. This is a conservative admission heuristic, not a measured optimum for every machine. A bounded best-two-of-three pilot compared 4 ms and 6 ms; 6 ms did not consistently improve larger cases and regressed one 65,536-file metadata case by about 18%. The repeated results below use 4 ms. + +Three forward/reverse/shuffled case orders cover fourteen fixture shapes, workers and pinned CPU allowances 1/3/10, ordinary listing and eager metadata output, and separately labeled 2 ms-per-directory latency controls. Each binary receives a discarded warmup and nine interleaved repetitions per cell; retain the fastest seven. The full three-variant experiment performed 9,180 oracle-correct invocations. Additional warm-pool and asymmetric child-latency controls performed 540 oracle-correct invocations. The two budget pilots are screening evidence, not acceptance runs. All builds completed before each timing batch. + +Wall time is measured inside the native fork/exec/wait launcher; Python creation and oracle checking are outside that boundary. Fixtures and captured output use tmpfs. No normalization is applied between these same-host paired controls. The retained adaptive timing executable precedes later help-text-only changes; traversal/admission logic is identical. + +Negative elapsed change is faster. Values are medians of three per-session estimates; percentages are medians of the paired session changes. + +| Fixture / read model | Workers | Main ms | Fixed gate ms | Adaptive ms | Adaptive change | +| ---------------------------------------- | ------: | ------: | ------------: | ----------: | --------------: | +| broad8x1, warm | 3 | 4.270 | 0.902 | 0.873 | -79.6% | +| broad64x1, warm | 3 | 4.387 | 4.380 | 0.972 | -77.8% | +| broad8x512, warm | 3 | 5.093 | 1.842 | 1.802 | -64.6% | +| broad32x512, warm | 3 | 7.900 | 6.216 | 6.218 | -20.4% | +| broad64x512, warm | 3 | 11.874 | 11.769 | 12.183 | +2.3% | +| broad128x512, warm | 3 | 17.763 | 18.140 | 17.659 | -1.7% | +| broad8x1, 2 ms/open control | 3 | 13.132 | 20.174 | 13.181 | +0.4% | +| broad8x1, 2 ms/open control | 10 | 9.119 | 20.130 | 9.361 | +1.2% | +| broad63x1, 2 ms/open control | 10 | 22.034 | 134.430 | 22.008 | -0.2% | +| warm_pool_eager, 2 ms/child-open control | 3 | 11.908 | 22.385 | 11.897 | +0.7% | +| warm_pool_eager, 2 ms/child-open control | 10 | 8.116 | 22.821 | 8.215 | +1.3% | +| child_only8, 2 ms/child-open control | 3 | 11.078 | 18.067 | 13.262 | +16.8% | +| child_only8, 2 ms/child-open control | 10 | 7.179 | 18.197 | 9.250 | +29.4% | +| child_only32, 2 ms/child-open control | 3 | 27.839 | 68.023 | 29.967 | +7.4% | + +The fixed gate shows large small-scan gains but delays profitable slow reads even after the pool exists. Adaptive admission retains the cheap-scan benefit and stays near main for uniform slow-directory controls and existing-pool reuse. Across the 102 primary case definitions, the largest median adaptive regression was 2.3%; that does not establish a universal bound. + +**Remaining tradeoff:** a cheap parent does not predict expensive children. When the first child must be sampled synchronously, the candidate pays one child latency before starting parallel reads. The asymmetric eight-child control regresses about 17-30% against eager main. This is an explicit limitation, not an accepted no-regression claim. Heterogeneous later children, real slow storage and native macOS performance remain acceptance gaps; native macOS unit/integration tests establish behavior only. Keep #991 draft while those performance tradeoffs are reviewed. + +Separate uninstrumented timing and instrumented diagnostics. On the small warm batch, main created three workers and made 26 futex calls; the old inline gate made none. Main requested 1 ms and 2 ms sleeps. Matching LLVM symbolization (after correcting executable-segment file offsets by `+0x1000`) places them in `MeasureTscFrequencyWithSleep`, reached through Abseil mutex enqueue/clock frequency initialization from `ReadPool::Run`. The guest lacks `tsc_freq_khz`. This is not evidence that thread creation itself costs 3 ms. Removing incidental clock initialization or changing pool synchronization remains a separate candidate, with single-worker controls required before adoption. + +Raw data, CPU usage, RSS, executable hashes and fixture/output identities: [directory-admission-20261031.json.gz](benchmarks/directory-admission-20261031.json.gz). Reproduction scripts, native timer extension, latency interposer and diagnostics are retained in `/tmp/xff-991-evidence/` on drogon and `/private/tmp/xff-991-evidence/` locally. The latency injection is a controlled model, not verified physical slow-storage evidence. + ## Abseil clock-only startup experiment on current main A clock-only experiment on `a721b98a306dd9021d31bbc485ae4c7c82ceba00` removes the @@ -2731,3 +2768,83 @@ results; all five macOS targets execute. Full CI remains required before merging The macOS samples, binary hashes, invocation settings, output oracles and probe source are retained in [`abseil-steady-clock-macos-20261031.json.gz`](benchmarks/abseil-steady-clock-macos-20261031.json.gz). + +## Directory admission after the clock fix: #991 reassessment (2026-11-01) + +**Recommendation: reject #991's current 4 ms admission policy.** Keep it draft; +retain main's scheduling while any replacement is evaluated separately. The clock-only +fix #1034 has removed the main source of the previous startup cliff. A small cheap-scan +benefit remains, but synchronous child sampling still delays useful parallel reads. +This rejects the measured policy, not the possibility of improving worker startup. + +The comparison uses current main `974d137026818bba1c1deaf6cfc4fffe676b314e` and +synchronized #991 `d5cffe57b957266dc14bb301643bfa4e0b05525a`, both with the merged +Abseil steady-clock setting and `--config=clang_release`. No threshold retuning or +other implementation changes were made. The macOS baseline is built at the original +#1034 head, whose complete Git tree is identical to the merged main tree; that +provenance and tree ID are retained in the raw evidence. #991 also carries an +independent libpfm source-query downloader repair. Preserve or extract that repair +if the admission implementation is discarded. + +Linux runs on drogon (Ryzen 9950X KVM, ten vCPUs), with tmpfs fixtures and CPU affinity +budgets 1/3/10. macOS runs on the Apple M5 Pro with warm APFS fixtures and worker +budgets 1/3/10, without CPU pinning. Results are within-platform paired comparisons, +not a comparison of the hardware or operating systems. Builds finish before timing. + +There are 72 ordinary listing/metadata/content cases per platform, over eight fixture +shapes spanning 10 through 65,536 files. Linux additionally has ten controlled +slow-read cases: uniform delayed directories, a cheap parent with slow children, +and reuse of an existing metadata pool. The 2 ms/opendir injection models latency; +it is not physical slow-storage evidence. Each case has a separate warmup per binary, +nine interleaved samples, the mean fastest seven, and three differently ordered +sessions. Native fork/exec/wait timing excludes Python startup, fixture construction +and output verification. All 9,240 invocations pass independent output SHA256 oracles. + +Negative time change is faster. Times are medians of session estimates; percentages +are medians of paired session changes, so ratios of displayed medians can differ. + +| Platform / case | Workers | Main ms | #991 ms | Time change | +| :--------------------------------- | ------: | ------: | ------: | ----------: | +| linux / broad8x1 / files | 3 | 0.822 | 0.608 | -25.94% | +| linux / broad8x512 / files | 3 | 1.472 | 1.357 | -7.24% | +| linux / broad128x512 / files | 3 | 12.761 | 12.889 | +1.07% | +| linux / broad128x512 / content | 3 | 51.053 | 50.554 | -1.00% | +| linux / uniform8 / files | 3 | 9.359 | 9.346 | -0.14% | +| linux / child_only8 / files | 3 | 7.325 | 9.361 | +27.79% | +| linux / child_only8 / files | 10 | 3.411 | 5.447 | +59.04% | +| linux / child_only32 / files | 10 | 9.628 | 11.688 | +21.55% | +| linux / warm_pool_eager / metadata | 3 | 7.753 | 7.858 | +1.13% | +| macos / broad8x1 / files | 3 | 4.210 | 4.116 | -3.06% | +| macos / broad8x512 / files | 3 | 6.311 | 6.798 | +7.32% | +| macos / broad128x512 / files | 3 | 33.388 | 33.447 | +0.29% | +| macos / broad128x512 / content | 3 | 820.325 | 819.365 | -0.12% | +| macos / nested8x8x64 / files | 3 | 6.857 | 7.465 | +8.85% | + +The Linux eight-file/three-worker gain is about 0.21 ms, down from the previous +pre-clock-fix experiment's approximately 3.4 ms saving. The asymmetric eight-child +case pays about 2 ms for its serial sample and regresses by 27.8% with three workers +or 59.0% with ten. Thirty-two slow children still regress by 8.6%/21.6% respectively. +Uniform slow reads stay near main, as does the existing-pool control. Ordinary +65,536-file workloads show little benefit from the admission policy. On macOS, three-worker broad and nested 4,096-file listings consistently regress +by 7.3% and 8.8% across all three sessions; metadata cases regress by 6.6% and +8.5%. The tiny eight-file listing gains about 3.1% (roughly 0.1 ms), while large +scans stay near main. These distinct populations must not be averaged into an +unconditional improvement claim. + +A separate small-scan trace creates three workers on main and none under #991; +neither trace requests the 1 ms/2 ms calibration sleeps. The remaining cheap-scan +benefit is avoided pool startup/scheduling, not removal of calibration. Main's +post-#1034 Test workflow `41247081300` succeeds. Output-oracle checks establish +correctness for these benchmark cases, not full PR readiness or slow-storage safety. + +Next work should reduce actual worker startup/queue cost or reuse a bounded run-owned +pool across eligible work. Preserve concurrency for small expensive frontiers; avoid +placing a synchronous sample before all parallel work. Do not choose a new threshold +by fitting this guest's measurements alone. JC07 stays open, including real slow +storage and heterogeneous children, ordering, exactly-once effects, cancellation, +metadata behavior, and cold versus warm dispatch. The first-child sample and the +4 ms constant should not be accepted merely because pre-clock-fix timings looked good. + +Raw samples, output identities, binary hashes, source/build provenance, reproduction +scripts, latency adapter and traces are retained in +[directory-admission-post-clock-20261101.json.gz](benchmarks/directory-admission-post-clock-20261101.json.gz). diff --git a/tools/repository_tooling_test.py b/tools/repository_tooling_test.py index 25a8e187edc..e96863be12c 100644 --- a/tools/repository_tooling_test.py +++ b/tools/repository_tooling_test.py @@ -69,6 +69,21 @@ def test_libpfm_downloader_rewrites_only_the_retired_archive(self): retired + "?download=1", canonical.removeprefix("https://")): self.assertIsNone(re.fullmatch(pattern, unrelated)) + def test_unconfigured_queries_repair_libpfm_without_enabling_counters(self): + commands = [shlex.split(line, comments=True) + for line in (self.workspace / ".bazelrc").read_text().splitlines()] + commands = [command for command in commands if command] + download_settings = [command for command in commands + if any(option.startswith("--downloader_config=") for option in command[1:])] + self.assertEqual(download_settings, [ + ["common:linux_perf", "--downloader_config=bazelmod/linux_perf_downloader.cfg"], + ["query", "--downloader_config=bazelmod/linux_perf_downloader.cfg"], + ]) + query_settings = [option for command in commands if command[0] == "query" + for option in command[1:]] + self.assertNotIn("--define=pfm=1", query_settings) + self.assertNotIn("--config=linux_perf", query_settings) + if __name__ == "__main__": parser = argparse.ArgumentParser(description=__doc__) diff --git a/xff/cli/globals.cc b/xff/cli/globals.cc index 4d7f75244dc..372d97f8b2f 100644 --- a/xff/cli/globals.cc +++ b/xff/cli/globals.cc @@ -1023,7 +1023,9 @@ constexpr std::array kGlobals = std::to_array({ "short form `-j4` is also accepted. Every form accepts `all` in place of `N`. `-j 1` makes " "directory reads and `-exec ... ;` synchronous. At larger values, xff reads directories " "on up to `N` worker threads; large directory listings can split required eager metadata reads " - "across the same pool. Lazy and metadata-free listings do not start stat workers. " + "across the same pool. Directory startup uses observed read cost and remaining independent work; " + "small cheap batches can stay inline, while an existing pool is reused. `N` is a limit, not a " + "promise to parallelize every listing. Lazy and metadata-free listings do not start stat workers. " "Independent content predicates and rg searches also match in bounded " "parallel batches, " "with output kept in traversal order. Small initial batches, archive members, stateful expressions " diff --git a/xff/engine/BUILD.bazel b/xff/engine/BUILD.bazel index 589895d96c8..c2a667ab0a9 100644 --- a/xff/engine/BUILD.bazel +++ b/xff/engine/BUILD.bazel @@ -19,6 +19,23 @@ load("//xff:cc.bzl", "cc_binary", "cc_library", "cc_test") package(default_visibility = ["//visibility:private"]) +# Internal, pure startup admission arithmetic; durations are supplied by the coordinator. +cc_library( + name = "directory_read_admission_cc", + hdrs = ["directory_read_admission.h"], +) + +cc_test( + name = "directory_read_admission_test", + size = "small", + srcs = ["directory_read_admission_test.cc"], + deps = [ + ":directory_read_admission_cc", + "@googletest//:gtest", + "@googletest//:gtest_main", + ], +) + # Directory traversal over a vfs::FileSystem: a worker pool reads (readdir+lstat) # in parallel while a single coordinator thread visits in --sort order. cc_library( @@ -27,6 +44,7 @@ cc_library( hdrs = ["walk.h"], visibility = ["//xff:__subpackages__"], deps = [ + ":directory_read_admission_cc", "//xff/vfs:vfs_cc", "@abseil-cpp//absl/algorithm:container", "@abseil-cpp//absl/base:core_headers", @@ -143,9 +161,12 @@ cc_test( ":walk_cc", "//xff/vfs:local_fs_cc", "//xff/vfs:vfs_cc", + "@abseil-cpp//absl/base:core_headers", "@abseil-cpp//absl/functional:function_ref", "@abseil-cpp//absl/status", "@abseil-cpp//absl/status:statusor", + "@abseil-cpp//absl/strings", + "@abseil-cpp//absl/synchronization", "@googletest//:gtest", "@googletest//:gtest_main", "@mboworks_mbo//mbo/testing:status_cc", diff --git a/xff/engine/directory_read_admission.h b/xff/engine/directory_read_admission.h new file mode 100644 index 00000000000..5980dd9cc37 --- /dev/null +++ b/xff/engine/directory_read_admission.h @@ -0,0 +1,31 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#ifndef XFF_ENGINE_DIRECTORY_READ_ADMISSION_H_ +#define XFF_ENGINE_DIRECTORY_READ_ADMISSION_H_ + +#include +#include +#include +#include + +namespace xff::engine::walk_internal { + +// A conservative cold-start work budget, including pool synchronization. This is an admission +// estimate, not a deadline or a promise that sibling reads have identical cost. An existing pool +// does not need to amortize startup again. Keep the arithmetic overflow-safe for large listings. +constexpr bool UsefulDirectoryReads(std::chrono::nanoseconds sample, std::size_t directories, std::size_t workers) { + if (sample.count() <= 0 || directories < 2 || workers < 2) { + return false; + } + workers = std::min(workers, directories); + const std::size_t rounds = (directories / workers) + (directories % workers != 0 ? 1 : 0); + const std::size_t saved_reads = directories - rounds; + constexpr auto kStartupBudget = std::chrono::duration_cast(std::chrono::milliseconds(4)); + return static_cast(sample.count()) + >= ((static_cast(kStartupBudget.count()) - 1) / saved_reads) + 1; +} + +} // namespace xff::engine::walk_internal + +#endif // XFF_ENGINE_DIRECTORY_READ_ADMISSION_H_ diff --git a/xff/engine/directory_read_admission_test.cc b/xff/engine/directory_read_admission_test.cc new file mode 100644 index 00000000000..cdb098bf12a --- /dev/null +++ b/xff/engine/directory_read_admission_test.cc @@ -0,0 +1,55 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include "xff/engine/directory_read_admission.h" + +#include +#include +#include + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace xff::engine::walk_internal { +namespace { + +using ::testing::IsFalse; +using ::testing::IsTrue; + +struct DirectoryReadAdmissionTest : ::testing::Test {}; + +TEST_F(DirectoryReadAdmissionTest, RejectsUnavailableTimingAndInsufficientParallelism) { + EXPECT_THAT(UsefulDirectoryReads(std::chrono::nanoseconds(0), 64, 3), IsFalse()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::nanoseconds(-1), 64, 3), IsFalse()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::seconds(1), 0, 3), IsFalse()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::seconds(1), 1, 3), IsFalse()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::seconds(1), 64, 0), IsFalse()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::seconds(1), 64, 1), IsFalse()); +} + +TEST_F(DirectoryReadAdmissionTest, CheapWideListingsDoNotForceStartup) { + for (const std::size_t directories : {63, 64, 65, 128}) { + EXPECT_THAT(UsefulDirectoryReads(std::chrono::microseconds(5), directories, 10), IsFalse()); + } +} + +TEST_F(DirectoryReadAdmissionTest, SmallSlowBatchCanRepayStartup) { + EXPECT_THAT(UsefulDirectoryReads(std::chrono::milliseconds(2), 8, 3), IsTrue()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::milliseconds(2), 8, 10), IsTrue()); +} + +TEST_F(DirectoryReadAdmissionTest, AccountsForWorkerCountAndIncompleteLastRound) { + EXPECT_THAT(UsefulDirectoryReads(std::chrono::milliseconds(1), 6, 2), IsFalse()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::milliseconds(1), 6, 3), IsTrue()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::microseconds(600), 8, 100), IsTrue()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::microseconds(570), 8, 100), IsFalse()); +} + +TEST_F(DirectoryReadAdmissionTest, IncludesExactBudgetBoundaryWithoutMultiplicationOverflow) { + EXPECT_THAT(UsefulDirectoryReads(std::chrono::nanoseconds(3'999'999), 2, 2), IsFalse()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::milliseconds(4), 2, 2), IsTrue()); + EXPECT_THAT(UsefulDirectoryReads(std::chrono::nanoseconds(1), std::numeric_limits::max(), 2), IsTrue()); +} + +} // namespace +} // namespace xff::engine::walk_internal diff --git a/xff/engine/walk.cc b/xff/engine/walk.cc index 011657e3e70..1f803c8c610 100644 --- a/xff/engine/walk.cc +++ b/xff/engine/walk.cc @@ -16,6 +16,7 @@ #include "xff/engine/walk.h" #include +#include #include #include #include @@ -40,6 +41,7 @@ #include "absl/types/span.h" #include "mbo/status/status_macros.h" #include "mbo/types/optional_ref.h" +#include "xff/engine/directory_read_admission.h" #include "xff/vfs/entry.h" #include "xff/vfs/filesystem.h" @@ -80,6 +82,8 @@ class ReadPool { public: explicit ReadPool(std::size_t workers) : max_workers_(workers) {} + bool Started() const { return !threads_.empty(); } + // Start only when sibling directories or eager stat chunks provide independent work. void Start(std::size_t directories) { const std::size_t count = std::min(max_workers_, directories); @@ -325,7 +329,19 @@ class Walker { // Only the coordinator dispatches and joins stat chunks. Directory read-ahead jobs // stay leaves: no worker ever waits for jobs submitted to its own pool. + // Only the coordinator records timing, and only while startup is still avoidable. Workers read + // through ReadDir; no shared timing cache, timing mutex or per-entry clock reads are introduced. Listing ReadNow(const std::string& dir) { + if (options_.workers <= 1 || pool_.Started()) { + return ReadNowImpl(dir); + } + const auto begin = std::chrono::steady_clock::now(); + Listing listing = ReadNowImpl(dir); + last_directory_read_ = std::chrono::steady_clock::now() - begin; + return listing; + } + + Listing ReadNowImpl(const std::string& dir) { MBO_ASSIGN_OR_RETURN(auto entries, fs_.ReadDir(dir)); constexpr std::size_t kChunk = 128; if (options_.workers <= 1 || options_.metadata != MetadataDemand::kAlways || entries.size() < 4 * kChunk @@ -592,13 +608,32 @@ class Walker { if (directories.size() < 2) { return reads; } - pool_.Start(directories.size()); - for (const std::size_t index : directories) { + std::size_t first = 0; + if (!pool_.Started() && !UsefulDirectoryReads(directories.size())) { + // Sample one real child without rereading it; parent listing cost need not represent children. + const std::size_t index = directories.front(); + std::promise sampled; + reads[index] = sampled.get_future(); + sampled.set_value(ReadNow(children[index].path)); + first = 1; + if (!pool_.Started() && !UsefulDirectoryReads(directories.size() - first)) { + return reads; + } + } + pool_.Start(directories.size() - first); + for (; first < directories.size(); ++first) { + const std::size_t index = directories[first]; reads[index] = SubmitRead(children[index].path); } return reads; } + bool UsefulDirectoryReads(std::size_t directories) const { + return walk_internal::UsefulDirectoryReads( + std::chrono::duration_cast(last_directory_read_), directories, options_.workers); + } + + std::chrono::steady_clock::duration last_directory_read_{}; const vfs::FileSystem& fs_; const WalkOptions& options_; Visitor visit_; diff --git a/xff/engine/walk_test.cc b/xff/engine/walk_test.cc index fb545bad6f0..01ea44ba65f 100644 --- a/xff/engine/walk_test.cc +++ b/xff/engine/walk_test.cc @@ -16,6 +16,8 @@ #include "xff/engine/walk.h" #include +#include +#include #include #include #include @@ -25,13 +27,16 @@ #include #include #include +#include #include #include +#include "absl/base/thread_annotations.h" #include "absl/functional/function_ref.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" +#include "absl/synchronization/mutex.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "mbo/testing/status.h" @@ -51,12 +56,16 @@ using ::testing::IsTrue; namespace fs = ::std::filesystem; using ::mbo::testing::IsOk; +using ::testing::Contains; using ::testing::ElementsAre; using ::testing::IsEmpty; +using ::testing::Le; using ::testing::Lt; using ::testing::Not; using ::testing::Pair; +using ::testing::SizeIs; using ::testing::UnorderedElementsAre; +using ::testing::UnorderedElementsAreArray; // In-memory FileSystem for tests that need metadata the real filesystem won't // reproduce locally (device ids for -xdev, symlink targets / loops for -L/-H). @@ -134,6 +143,37 @@ class FakeFs : public vfs::FileSystem { std::map targets_; }; +class ReadThreadFs : public FakeFs { + public: + ReadThreadFs(std::chrono::milliseconds root_delay, std::chrono::milliseconds child_delay) + : root_delay_(root_delay), child_delay_(child_delay) {} + + absl::StatusOr> ReadDir(std::string_view dir) const override ABSL_LOCKS_EXCLUDED(mu_) { + { + const absl::MutexLock lock(mu_); + reads_.emplace_back(dir, std::this_thread::get_id()); + } + const auto delay = dir == "root" ? root_delay_ : child_delay_; + if (delay.count() > 0) { + std::this_thread::sleep_for(delay); + } + return FakeFs::ReadDir(dir); + } + + std::vector> Reads() const ABSL_LOCKS_EXCLUDED(mu_) { + const absl::MutexLock lock(mu_); + return reads_; + } + + private: + const std::chrono::milliseconds root_delay_; + const std::chrono::milliseconds child_delay_; + + // Guards reads_; inherited filesystem nodes are immutable throughout Walk. + mutable absl::Mutex mu_; + mutable std::vector> reads_ ABSL_GUARDED_BY(mu_); +}; + vfs::Entry DirEntry(const std::string& path, const std::string& name) { return vfs::Entry{.path = path, .name = name, .type = vfs::FileType::kDirectory}; } @@ -202,6 +242,108 @@ WalkAction Continue(const Visit& /*visit*/) { return WalkAction::kContinue; } +struct DirectoryReadAdmissionTest : ::testing::Test { + struct Scenario { + std::size_t directory_count = 8; + std::size_t workers = 3; + bool slow_root = false; + bool eager_root = false; + std::size_t files_per_directory = 1; + }; + + static void ExpectDirectoryReads(const Scenario& scenario) { + const auto [directory_count, workers, slow_root, eager_root, files_per_directory] = scenario; + ReadThreadFs filesystem(std::chrono::milliseconds(slow_root ? 10 : 0), std::chrono::milliseconds(10)); + // A lower bound on observed work makes worker-admission checks independent of machine speed. + // Timing-free policy boundaries are tested separately in directory_read_admission_test.cc. + std::vector children; + std::vector> expected{{"root", 0}}; + for (std::size_t index = 0; index < directory_count; ++index) { + const std::string name = absl::StrCat("dir-", index); + const std::string path = absl::StrCat("root/", name); + children.push_back(DirEntry(path, name)); + expected.emplace_back(path, 1); + std::vector files; + for (std::size_t file_index = 0; file_index < files_per_directory; ++file_index) { + const std::string file_name = absl::StrCat("file-", file_index); + const std::string file_path = absl::StrCat(path, "/", file_name); + files.push_back(FileEntry(file_path, file_name)); + filesystem.AddFile(file_path, 1); + expected.emplace_back(file_path, 2); + } + filesystem.AddDir(path, 1, std::move(files)); + } + if (eager_root) { + // ReadNow starts the existing metadata pool before directory admission is considered. + for (std::size_t index = 0; index < 512; ++index) { + const std::string name = absl::StrCat("file-", index); + const std::string path = absl::StrCat("root/", name); + children.push_back(FileEntry(path, name)); + filesystem.AddFile(path, 1); + expected.emplace_back(path, 1); + } + } + filesystem.AddDir("root", 1, std::move(children)); + const std::thread::id coordinator = std::this_thread::get_id(); + std::vector> seen; + int errors = 0; + const absl::Status status = Walk( + filesystem, {"root"}, WalkOptions{.workers = workers}, + [&](const Visit& visit) { + seen.emplace_back(visit.path, visit.depth); + EXPECT_THAT(std::this_thread::get_id(), coordinator); + return WalkAction::kContinue; + }, + [&](std::string_view, absl::Status) { ++errors; }); + EXPECT_THAT(status, IsOk()); + EXPECT_THAT(errors, 0); + EXPECT_THAT(seen, UnorderedElementsAreArray(expected)); + const auto reads = filesystem.Reads(); + EXPECT_THAT(reads, SizeIs(directory_count + 1)); + EXPECT_THAT(reads, Contains(Pair("root", coordinator))); + std::size_t inline_children = 0; + for (const auto& [path, thread] : reads) { + if (path != "root" && thread == coordinator) { + ++inline_children; + } + } + if (workers <= 1) { + EXPECT_THAT(inline_children, directory_count); + } else if (slow_root || eager_root) { + EXPECT_THAT(inline_children, 0); + } else { + // At most one child is sampled inline; its ready future must not cause a second read. + EXPECT_THAT(inline_children, Le(1)); + } + } +}; + +TEST_F(DirectoryReadAdmissionTest, SmallSlowBatchStartsWorkersFromObservedParentCost) { + ExpectDirectoryReads({.slow_root = true}); +} + +TEST_F(DirectoryReadAdmissionTest, SmallSlowBatchSamplesChildWhenParentIsCheap) { + ExpectDirectoryReads({}); +} + +TEST_F(DirectoryReadAdmissionTest, ReusesMetadataWorkersForSmallDirectoryBatch) { + ExpectDirectoryReads({.eager_root = true}); +} + +TEST_F(DirectoryReadAdmissionTest, SampledChildCanActivateMetadataWorkers) { + ExpectDirectoryReads({.files_per_directory = 512}); +} + +TEST_F(DirectoryReadAdmissionTest, SlowBatchesHaveNoSixtyFourSiblingCliff) { + for (const std::size_t count : {63, 64, 65}) { + ExpectDirectoryReads({.directory_count = count, .workers = 8}); + } +} + +TEST_F(DirectoryReadAdmissionTest, OneWorkerStaysOnCoordinatorForSlowReads) { + ExpectDirectoryReads({.workers = 1, .slow_root = true}); +} + TEST_F(WalkTest, VisitsWholeTreePreorder) { const Result result = Run(WalkOptions{}, Continue); EXPECT_THAT(result.status, IsOk());