Skip to content

perf(graph): do not rebuild TestFileFilter when the session already has one - #836

Merged
jonathanong merged 4 commits into
execute-plan/79dd0742-pr-5-normalized-path-membership-without-pathcmpfrom
execute-plan/79dd0742-pr-6-reuse-prepared-testfilefilter-during-graph-config
Sep 5, 2026
Merged

perf(graph): do not rebuild TestFileFilter when the session already has one#836
jonathanong merged 4 commits into
execute-plan/79dd0742-pr-5-normalized-path-membership-without-pathcmpfrom
execute-plan/79dd0742-pr-6-reuse-prepared-testfilefilter-during-graph-config

Conversation

@jonathanong

@jonathanong jonathanong commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

An AnalysisSession owns one TestFileFilter per root. Graph config reuses it instead of calling TestFileFilter::newproject_filters again. Standalone DepGraph::build_with_plan_and_config still uses the unprepared fallback (session=None).

Call-count tests (not value equality) lock: seeded session does not increment test_filter.builds / project_filters; unseeded session builds once; two roots do not share a cell; session=None still compiles project filters without a session build.

New bench group graph_gates_build_session (does not change the existing graph_gates_build identity).

Local Criterion (source of truth)

Machine: macOS aarch64, rustc 1.96.0, --sample-size 20. Same machine before/after. CodSpeed is not used as a gate.

Unprepared graph_gates_build still compiles test globs once per build (path unchanged; movement is noise):

bench before (mean) after (mean)
graph_gates_build/1 57.5 ms 52.8 ms
graph_gates_build/4 56.9 ms 46.5 ms

Prepared session path vs unprepared before on the same machine:

bench unprepared before session after vs unprepared
graph_gates_build_session/1 57.5 ms 26.0 ms −55%
graph_gates_build_session/4 56.9 ms 35.8 ms −37% (high variance on [4])
Shepherd Journal
  • Split graph_gates.rs session bench and builder.rs session entry into sibling modules to satisfy rust-max-lines-per-file (4dcd916). CodSpeed extract regression used unexpected base main and different runtime environments; local Criterion is the gate.
  • Codex quota IC_kwDOSaSRXM8AAAABStTVkA/IC_kwDOSaSRXM8AAAABStTW5A and Sourcery budget review PRR_kwDOSaSRXM8AAAABMTLc_A are bot-limit notices; minimize only.
  • codecov javascript "No report found to compare against" and rust "No coverage information found on head" while jobs are still inProgress are stacked-PR upload lag, not a TestFileFilter defect.
  • Rejected Codex P2 PRRT_kwDOSaSRXM6fhdbC: AnalysisSession is request-scoped with one effective config (see the method comment and prepared-analysis ownership). A different config_path is a new request/session. Keying the OnceLock by root matches visible-path cells; hashing config identity on every graph-gates lookup would undo the reuse this PR measures. codecov patch/project lag is still upload/inProgress, not a missing TestFileFilter unit.
  • codecov/patch/rust 55.47% is unchanged after tests finished (35 passing). Diff includes benches/graph_gates_session.rs which is not in the coverage binary; production TestFileFilter reuse is covered by files_config_session_* tests. Project javascript/rust "no report to compare" is stacked-PR upload lag against perf: probe visible/inventory paths with OsStr order, not Path components #835. No additional production code change.
  • codecov/patch/rust 55.47% was missing coverage of builder_session.rs. Added files_config_session_builder_reuses_prepared_test_filter so the session DepGraph builder path is executed twice and reuses the prepared TestFileFilter.
  • Codex quota comment IC_kwDOSaSRXM8AAAABStc8SQ is a bot-limit notice; minimize only.
  • Benchmark (CPU graph gates) on run 33955328154 failed installing CodSpeed CLI 5.2.1 (curl: (35) Recv failure: Connection reset by peer) — transient infra, not graph-gates session code. codecov/patch/rust rose from 55.47% to 82.48% after the session builder test; remaining gap is likely uninstrumented benches/graph_gates_session.rs. Attempted rerun of 33955328154.
  • Reran 33955328154 once: CodSpeed CLI install curl reset is transient infra. codecov/patch/rust 82.48% remaining gap is benches/graph_gates_session.rs (not in the coverage binary), not missing production TestFileFilter tests.

@coderabbitai

This comment has been minimized.

@sourcery-ai

This comment has been minimized.

@codecov

This comment has been minimized.

@codspeed-hq

This comment has been minimized.

@jonathanong
jonathanong force-pushed the execute-plan/79dd0742-pr-6-reuse-prepared-testfilefilter-during-graph-config branch from 359385f to 50f8de2 Compare September 5, 2026 07:06
@jonathanong
jonathanong force-pushed the execute-plan/79dd0742-pr-6-reuse-prepared-testfilefilter-during-graph-config branch from 4dcd916 to ddc0f29 Compare September 5, 2026 07:40
@jonathanong
jonathanong force-pushed the execute-plan/79dd0742-pr-6-reuse-prepared-testfilefilter-during-graph-config branch from ddc0f29 to 4a81884 Compare September 5, 2026 07:47
@jonathanong
jonathanong force-pushed the execute-plan/79dd0742-pr-6-reuse-prepared-testfilefilter-during-graph-config branch from 4a81884 to a6314e9 Compare September 5, 2026 07:51
…as one

Thread prepared project filters into graph config load so a session does not
compile test globs a second time.
Add multi-root and unprepared-fallback call-count tests; trim comments.
rust-max-lines-per-file capped builder.rs and graph_gates.rs at 200 code
lines after the session TestFileFilter path was added.
@jonathanong
jonathanong force-pushed the execute-plan/79dd0742-pr-6-reuse-prepared-testfilefilter-during-graph-config branch from a6314e9 to 4ddb5c0 Compare September 5, 2026 07:56
@jonathanong
jonathanong marked this pull request as ready for review September 5, 2026 07:56
@chatgpt-codex-connector

This comment has been minimized.

sourcery-ai[bot]

This comment was marked as resolved.

@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

builder_session.rs was split out for the line-limit gate and was missing
from the files_config tests, so Codecov patch coverage of the session
builder stayed below the 99% target.
@chatgpt-codex-connector

This comment has been minimized.

@jonathanong
jonathanong merged commit 74342e6 into main Sep 5, 2026
69 of 73 checks passed
@jonathanong
jonathanong deleted the execute-plan/79dd0742-pr-6-reuse-prepared-testfilefilter-during-graph-config branch September 5, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant