Repository navigation
Conversation
This was referenced Oct 10, 2026
Address all six clang-tidy diagnostics in Walker and RunExecutorTest. Explicitly forbid copying or moving a Walker whose drain jobs capture it, use const observers where possible, and reserve the bounded task vector. The matcher intentionally admits workers early when its timed prefix is slow. Test the four-worker budget and eventual accumulated-work admission without assuming sanitizer or scheduler speed. Keep runtime thresholds and downstream caller-inclusive budgeting unchanged.
This was referenced Oct 10, 2026
helly25
force-pushed
the
perf/directory-read-pool-threshold
branch
from
October 10, 2026 16:37
3784053 to
38210c8
Compare
This was referenced Nov 1, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reuse directory and matcher workers across traversal phases to reduce fixed scheduling costs. This draft explores useful-work admission and batch scheduling; command-wide ownership and performance acceptance remain unfinished.
AG;DR
This draft is stacked on PR #991 (
perf/directory-read-pool-threshold). It changes the engine scheduler and records the native audit findings; performance acceptance remains pending.CI repair (2026-10-10)
Commit
1358d1fc4dcf942581036076ea1dc4fa731f82ccfixes the failures in run38001374066without changing runtime admission thresholds or folding downstream PRs into this change.run_executor_test,parallel_match_test,walk_test,walk_parallel_test,parallel_compare_test, andrun_test, under--config=clang.Scheduling and ownership
RunExecutor, capped by the configured worker limit. Pool growth preserves existing threads; each traversal joins them at teardown. This is not yet a complete command-wide concurrency cap; comparison ownership is an explicit remaining gap below.Completion and lifetime
AccumulatedWorkStartsWorkersWithinBudget, checks the actual four-worker upper bound during the first seven batches, and still requires four workers by the accumulated-entry threshold. It does not change the count/time policy, adopt downstream caller-inclusive budgeting, or skip sanitizer coverage. Separate remaining-work scheduling work is tracked in draft PR Plan content workers from remaining batch cost #1001 and is not part of this fix.Self-documentation and audit evidence
--jobshelp registry, regenerateXFF.md, and document shared executor ownership and batch scheduling indocs/design-parallel.md.docs/benchmark-correctness-audit.mdwith the native Linux findings and explicit remaining controls.xff-perf-shared-executor-v2run from acceptance: local correctness builds overlapped measurement, and its source predates this final scheduler.CycleClock::Frequency, which triggers Abseil's TSC calibration with 1 ms and 2 ms requested sleeps on this host, wheretsc_freq_khzis absent. This is one-time clock initialization, not a per-thread pthread creation cost. The provisional 500-microsecond cold-worker budget is not a measured startup model and remains unaccepted. Retain 36 profile cells (7,596 oracle-checked fresh invocations) at/tmp/xff-native-startup-profile-session1and eight sleep-stack controls at/tmp/xff-native-calibration-trace-session1; instrumented durations are not acceptance timings.978f67355a6ebcd7362db1a995e70c143b3abca4de1014daa5d8d003d13c5909. It disables Abseil's optional unscaled clock with both target and host C++ flags, without changing default settings. Three independently materialized ascending/descending/shuffled sessions have completed at/tmp/xff-clock-control-repeated-session1: 378 cells, 10,206 raw samples, all 11,340 invocations oracle-correct. Median paired changes are -60.3% at broad 20/jobs3 and -51.0% at deep 2k/jobs3; tiny broad 10/jobs1 consistently regresses +3.2%. Twelve native mechanism controls confirm the parallel variants create the same two workers, but only baseline requests the 1 ms and 2 ms calibration sleeps. Documentation-only PR #995 retains the complete raw timing and diagnostic evidence. These use native accounting and must not be pooled with historical Python-launch timings. This isolates a mechanism; it does not validate the executor or its admission policy. Apple already defaults this optional clock off; native macOS performance controls still remain required.Validation
bazel test //xff/engine:run_executor_test //xff/engine:parallel_match_test //xff/engine:walk_test //xff/engine:rg_test //xff/engine:run_test //xff/cli:globals_test //xff/cli:xff_markdown_test --config=clang --config=xff_docs.--config=clang --config=tsanand with--config=clang --config=asan(including UBSan). The initial five-suite explicit-completion TSAN run also passes twice.git diff --checkpasses./tmp/xff-perf-shared-executor-clean-v2, batch71f0060259ffdc5e36dce9202e107c6bfe1bc39c24a6f00b399d2b717e37006f, completed successfully. All three builds completed before measurement: baseline4ea895f631, Evaluate directory worker admission after the clock fix #99137840530d1, and this PR2c56ec76c9. The frozen candidate SHA-256 is3c4c0622afe15f98482e41bbc1329805c0f22e1a30e008c71200a3a2a141822c. The contract includes all 13 file counts, workers 1/3/10, both v2 layouts, nine repetitions, fastest seven, verified tmpfs and recorded CPU affinity. Guest topology/affinity does not independently establish host physical-core pinning. This is not performance acceptance./tmp/xff-audit-clean-summary.json. This is one sequential session, not repeated/reordered acceptance. For example, broad 20-file enumeration with three workers changes from 4.787 ms on baseline to 1.407 ms in Evaluate directory worker admission after the clock fix #991 and 1.447 ms in the frozen executor candidate; deep 2,000-file enumeration changes from 5.507 ms to 2.341/2.211 ms. However, the frozen candidate's broad 5,000-file content-needle case is 11.649 ms versus baseline 10.131 ms, with rg 11.048 ms and a 15.1% normalized regression. Downstream admission changes are not measured by this collection.Portability and acceptance limits
Hosted reports for the frozen candidate have content-regression alarms on both platforms despite the non-blocking benchmark check passing. In particular, macOS broad content-needle at 5,000 files and three workers measures 64.02 ms versus main's 33.45 ms, while rg measures 57.29 ms versus 60.31 ms. Linux reports content alarms at 2,000 and 5,000 files. These reports use the legacy grid rather than the native v2 fixtures. They contradict performance acceptance and motivate the cost-aware admission experiment; they do not validate that follow-up. Retained hosted artifacts are from run
38001374066,benchmark-pr-macos-arm64andbenchmark-pr-linux-x86_64.The implementation uses portable C++ and existing Abseil dependencies. PR #991's hosted macOS results do not validate this follow-up. Keep this PR draft until its Linux/macOS CI and benchmark controls have been examined, clean complete-grid repeated A/B timings preserve output hashes, and content, eager-metadata and slow-storage controls show no material regressions. The audit additionally calls for native M5 Pro sampling/A-B evidence; it is not supplied by a green hosted check.
The separate 20k/50k/100k single-worker slope investigation remains open. PR #995 now retains six actual baseline enumeration allocation profiles and independent native-launcher RSS controls on both full-grid v2 layouts. All twelve invocations pass the original oracle and before/after fixture/binary checks. At 100k, broad peaks at 0.311 MiB tracked heap/5.531 MiB native RSS, while deep reaches 34.006/40.191 MiB despite similar allocation counts. Deep global-peak stacks implicate path construction and listing vectors retained across recursive descent. This is evidence for a listing/path-lifetime investigation, not proof of CPU contribution or an accepted optimization. No claim is made that executor reuse fixes that per-entry slope or that it improves every workload. Historical forked-Python RSS remains unsuitable for choosing arena sizes; tracked heap and corrected native RSS are separate metrics.
Two scheduling concerns need explicit controls before acceptance: directory drains currently run to the end of their sibling batch and can occupy every worker ahead of queued content or stat work; the frozen candidate's cold matcher admission counts previously processed entries, which does not prove that enough remaining work will repay startup for a small rendered-content batch. Downstream PR #1001 addresses remaining-work admission separately; it is not part of this PR and does not by itself establish optimal gates or queue fairness. Thread reuse alone is not a throughput or fairness result.
Source inspection also identifies a command-wide ownership gap: comparison unconditionally starts one traversal with
std::asyncand runs the other on the caller. EachRunFindCoreowns its own N-worker executor, so two coordinators and two N-worker pools can coexist;--jobs=1still starts the extra coordinator. After traversal,ParallelCompareowns a separate pool instead of reusing either executor. The two comparison coordinators predate this PR, but the draft does not repair their global budget or phase reuse. Read-ahead also lacks a command-wide retained-byte budget, and already queued unused reads drain after quit. A follow-up must explicitly account for coordinator participation, share command ownership across comparison phases, bound the frontier and retained bytes, and fairly schedule substantial batches. The current executor assumes one coordinator and cannot simply be shared between concurrently calling coordinators. These remain unresolved; the draft is not acceptance-ready.PR #995 additionally retains 24 large-grid, single-worker CPU diagnostic cells for enumeration, tree sorting, collection and content. All 1,440 fresh invocations pass the oracle, with nine direct counter samples and fifty recorded invocations per cell. Plain enumeration's user instructions/file remain approximately 2.5k-2.6k at 20k/50k/100k on both layouts, despite the deep live-heap increase. These are diagnostic workload controls, not scheduler A/B or precise phase costs. Corrected sample accounting excludes the Python recording driver's period from the XFF denominator while retaining kernel-context IPs as unattributed. Original profile hashes and complete reports/scripts are retained; native M5 Pro and scheduling acceptance controls remain open.
The exact initial-source Linux baseline is now freshly built and complete at
772994ab7905be77915978f06ea90ae5934aaeef, producing the same retainedf97833...executable. All 858 full-grid task cells, 14,040 retained samples and 1,560 warmups pass; a separate verifier checks source/configuration, binary, frozen driver/helper, fixture/oracle identities, affinity, sample/resource metrics and estimates. PR #995 retains exact original report bytes, the full analysis and provenance. With the corrected native helper, jobs3 enumeration reproduces broad 10/20 at 1.842/5.241 ms (rg 3.103/3.230) and deep 1k/2k at 2.386/6.151 ms (rg 3.223/3.923). This supplies the exact-source Linux baseline, not acceptance of this candidate, native Mac evidence or repeated/reordered controls. No old Python/native timing populations are pooled.Current qualification disposition (2026-11-07)
This proposal remains open as unqualified research; its branch and implementation are preserved. The successful checks above belong to its historical head/base context. They are not validation against current main, and this note changes no source, branch, base, CI policy or acceptance threshold.
PR #1125 has merged as
9bf1aad510d843d1d6338fbdf23107a30d339730. Its shared-stage-worker report rejects six separately implemented reuse designs, including a repeatable 15.55% Linux small-identical/1,000-pair/four-worker regression. That result does not establish that every unique mechanism in #992/#996/#997/#998 is rejected or superseded. Main retains specialized pools.This PR introduces the shared executor API consumed by #996. Its original accumulated-past-work/8,192-entry and 500-microsecond matcher admission remains a known harmful policy. The independent eager-only control in the executor/copy audit removes the worst content losses, but rg selection at 1,000 entries/four workers still costs 19.8% on Linux and 5.7% on macOS. Reuse alone has not passed the counterexamples.
The GitHub base is the closed/rejected #991 directory-admission branch, and the current PR is conflicting. Do not mechanically refresh the historical stack or import that rejected gate. A viable successor needs an isolated current-main executor integration, preserved independent stage/root/mounted-walk allowances, leaf jobs and owner draining, then complete paired native and in-memory content/comparison controls on both platforms. Keep the existing role-specific pools until such a successor is qualified.