fold/docs/research/optimization-loop-2026-07-30.md
trvon 425eeb0751
Some checks failed
build / build-linux (push) Failing after 7m50s
build / build-macos (push) Has been cancelled
build / release (push) Has been cancelled
docs: record option-2 S0 result — candidate expansion is empty on fold/yams
Measured (scratch, reverted): with the graph-fed cross-TU candidate
expansion disabled, finding fingerprints stay exact (fold 0fec78e5...,
yams 247523c5..., status ok) and securityGraphCandidateFunctions
returns an empty set (expanded=0) for these manifests. The 18.8s graph
build is pure overhead for the expansion path here. Remaining graph
consumer is summary priming; S0b tests whether its SCC-ordered eager
prepass can be dropped in favor of the lazy on-demand summary path.
2026-09-01 17:32:40 -06:00

10 KiB
Raw Permalink Blame History

Optimization Loop — Decision Record

Date: 2026-07-30 Status: accepted (workflow active) Grounding: project-eval-scaffolding-2026-07-27.md (manifests, gates), PROMPT-eng-codex.md (retrieval contract, execution loop, handoff), PROMPT-yams-cpp-workflow.md (design-first TDD, measurement-first profiling).

Decision

Performance and bug-finding optimization on fold becomes a phased, evidence-first loop driven by the existing benchmark infrastructure, not ad-hoc intuition driven tuning:

measure -> select -> implement -> validate -> checkpoint -> measure ...

Each phase is a persisted workflow state with a declared outcome, so an optimization campaign survives session restarts and rework (validate failure returns to implement, never to measure) without losing evidence.

Phase contracts

1. measure

  • Build release first: ninja -C build/release.

  • Run the profiled harness over the self-hosting manifests:

    python3 scripts/benchmarks/run_optimization_harness.py \
      --manifest tests/benchmarks/projects/fold_source.json \
      --out-dir tests/benchmarks/runs/optimization-<date>
    python3 scripts/benchmarks/run_optimization_harness.py \
      --manifest tests/benchmarks/projects/yams_source.json \
      --out-dir tests/benchmarks/runs/optimization-<date>
    
  • Record: per-detector wall time, RSS, finding count, stability (max_churn=0, pairwise Jaccard=1.0), token/report bytes.

  • Outcome evidence-ready when at least one run artifact exists with the profile rows intact; evidence-failed if the harness cannot run (fix the environment before optimizing anything).

2. select

  • Pick exactly one target per iteration from measured evidence:
    • a hotspot (top per-detector wall or RSS row), or
    • a bug-finding quality gap (FP churn, missed class, unstable fingerprint).
  • State the KPI and the expected direction before touching code (PROMPT-yams-cpp-workflow rule 5: never optimize from intuition).
  • Outcome target-picked with KPI written into the task/workflow state; outcome no-target (terminal) when no measured target clears the improvement bar this round.

3. implement

  • Design-first TDD per PROMPT-yams-cpp-workflow.md rules 2–4: smallest testable seam, Catch2 behavior test red before the production change, assertions per the fold policy in AGENTS.md/fold-assertion-policy.
  • One change per iteration. Hybrid/router/backend boundary rules from AGENTS.md apply unchanged.
  • Outcome change-made.

4. validate

Gates, in order, fastest first:

  1. git diff --check

  2. Focused tests for the touched area (meson test -C build/release fold:<area>).

  3. Full suite for CLI/router/backend changes (meson test -C build/release --print-errorlogs).

  4. Statistical A/B against baseline:

    python3 scripts/benchmarks/run_ab_evaluation.py \
      --baseline <prev-release-or-pre-change-fold> --current build/release/src/cli/fold
    

    Comparison uses fold_eval.metrics.paired_comparison when paired samples exist (single medians are explicitly labelled median-only, never treated as a regression oracle).

  5. Stability gate: finding fingerprints unchanged unless the change is a deliberate detector-behavior change (then dispositions must be updated and reviewed in the same iteration).

Hard failure rules: wall/RSS regression beyond the manifest's wall_regression_pct/rss_regression_pct, churn > 0, or Jaccard < 1.0 without reviewed dispositions → outcome gate-failed → back to implement (revert or fix; never weaken a gate to pass).

5. checkpoint

  • Index changed files and a handoff note into YAMS with the PROMPT-eng-codex.md metadata contract (task=optimization-loop, phase=complete, owner=opencode, source=note).
  • Commit with measured before/after numbers in the body; git push only with operator confirmation.
  • Update the baseline artifacts under tests/benchmarks/projects/baselines/ when a sustained win is accepted.
  • Outcome checkpointed → loop to measure.

Artifacts per iteration

Artifact Location
Profiled run evidence tests/benchmarks/runs/optimization-<date>/*.run.json
A/B comparison harness stdout + run dir summary
Decision note YAMS (source=note) + commit message
Baseline update tests/benchmarks/projects/baselines/ (only on accepted wins)

Iteration ledger (2026-07-31 campaign)

Iter Target Result
1 sql-injection hotspot profiling Characterization test fixed (UAF); context-reuse attempt contaminated
2 Red evaluation gate Dispositions re-baselined; both manifests pass
3 Call-graph fallback Merge duplicate tolerance fix: 24.1s → 1.7s (93%)
4 Broken schema test Full suite 137/137
5 Summary flake 3 rig attempts, not reproduced; rig committed
6 Z3 instrumentation Solver-setup hypothesis falsified (154 solvers, 6ms)
7 Scratch context re-attempt Accepted; all gates green
8 Summary-path sizing 66,147 constructions/scan eliminated
9 Feasibility memo Measured dead end (3.5% duplicates)
10 Dual-walk Measured dead end (0% fallback rate)

Cumulative: fold self-scan wall 251.7s → 86.2s (66% reduction), finding fingerprints exact at every step.

Deferred: in-pass graph contribution collection (design, 2026-07-31)

The no-index exhaustive call-graph build (~16.9s on the fold manifest) is a full ClangTool re-parse of all TUs (buildProjectCallGraph). The TU ASTs already exist in the main detector pass, and graphToContribution(database, ctx, ...) (internal in source_call_graph_contribution.cpp) builds a complete contribution from a live ASTContext. Planned change (next campaign slice):

  1. Expose buildContributionFromContext(...) (public wrapper around graphToContribution) in source_call_graph.hpp.
  2. In the main TU pass (source_tu_runner Consumer::HandleTranslationUnit), build each TU's contribution post-detectors and accumulate it in SourceTuRunState; the Consumer needs a CompilationDatabase& for fingerprinting (makeTranslationUnitFingerprint uses compile commands).
  3. Extend assembleProjectCallGraph with an optional prebuilt contributions parameter: when they cover the shard's files AND no project index is open, go straight to mergeTranslationUnitGraphContributions (skipping load+collect+persistence); all other paths unchanged.
  4. Fanout topology: contributions are per-shard; each shard's assembly covers its own files — consistent with today's per-shard exhaustive fallback.

Equivalence argument: graphToContribution is the same function the ClangTool collector calls with the same compilation database and configuration profile; the main-pass ASTs come from the same compile commands, so contributions should be byte-identical. The finding fingerprint gate (exact) decides acceptance; the exhaustive fallback is preserved as the worst case.

Estimated: ~80-150 lines across source_call_graph.{hpp,cpp}, source_tu_runner.{hpp,cpp}, project_graph_orchestrator.{hpp,cpp}, scan_runner.cpp.

INVALIDATED (iteration 12, 2026-07-31): implementation began and collapsed on an ordering dependency — assembleProjectCallGraph runs BEFORE the main TU pass because the TU pass consumes the graph (graph_security_candidates expands security candidates across TUs; symbolic summaries are primed from the call graph). In-pass collection cannot serve the same scan: the TU pass needs the graph the contributions would feed. Reversing the order breaks candidate expansion (fingerprint changes), and keeping every TU AST alive across both passes is infeasible (~1.4 GiB RSS for the fold self-scan). The only sound reductions for the no-index build are persistence (cache-dir, already exists) or a lighter graph parse — neither acceptable. The ~16.9s no-index build is the inherent cold-run price of cross-TU facts; closed as a target.

Non-goals

  • No gate weakening, no detector-disabling to reduce runtime, no silent coverage caps (router rules in AGENTS.md).
  • Comparators (Clang SA, sanitizers) stay differential evidence, never ground truth.
  • Binary-lane heuristics stay heuristic; performance work on the binary path never converts into source-coverage claims.

Option 2 scoping (2026-08-31, README-documented)

Decision: replace the per-scan double parse (graph pass + detector pass, call_graph_ms 18.8s on the fold self-scan) with single-pass graph building. Chosen over PCH/module caching to preserve portability (README 'Performance architecture'): no clang-version-locked artifacts, no SDK-locked module cache, no flag-matching bug surface.

Dependency map (who consumes the graph inside the detector pass):

  • primeCrossTUProperties (scan_runner.cpp) — third parse, only for div-by-zero/null-deref/int-overflow/oob; no-op on the fold manifest.
  • assembleProjectCallGraph -> call_graph -> graph_security_candidates feeding the security walk's candidate expansion (prunes 2711 of 6480 functions — a real perf win that must be preserved).
  • summary priming consumes the graph for SCC-ordered callee-first passes.

Tension to resolve before implementation: the candidate-expansion closure needs the full cross-TU graph, which exists only after every TU is parsed, so "run detectors in the same single pass" holds only for graph-INDEPENDENT detectors. Graph-dependent detectors require either (A) keeping the ASTs alive across the deferred closure (memory: ~210MB/AST, must stay under the existing 1.68GB peak profile), or (B) re-materializing the walk input from persisted IR — the exact equivalence proof is the work, policed by the finding-fingerprint + gold gates.

Option 2 S0 result (2026-08-31)

Measured (scratch, reverted): disabling the graph-fed cross-TU candidate expansion on fold + yams produced IDENTICAL finding fingerprints (fold 0fec78e5..., yams 247523c5..., status ok, gold gates green) with expanded=0 in every run — securityGraphCandidateFunctions returns an empty set for these manifests' detector mixes, so the graph's candidate- expansion consumer is finding-neutral/empty here. The 18.8s graph build is therefore pure overhead for the expansion path on these corpora.

Remaining graph consumer not yet tested: summary priming (SCC-ordered eager prepass). S0b: disable the eager primeSymbolicSummaries graph-SCC layering (rely on the lazy on-demand ensureSymbolicSummary path) and compare fingerprints — if exact, the graph is fully dispensable on fold+yams and option 2 collapses to "skip the graph build unless a corpus's detectors actually consume it" (lazy, equilibrium fallback to the full graph when a corpus needs expansion or SCC-ordered priming).