Skip to content

perf: add syntactic prefilters before semantic-model calls in all affected analyzers#463

Draft
dnyw4l3n13 wants to merge 1 commit into
mainfrom
perf/457-syntactic-prefilters-before-semantic-model-calls
Draft

perf: add syntactic prefilters before semantic-model calls in all affected analyzers#463
dnyw4l3n13 wants to merge 1 commit into
mainfrom
perf/457-syntactic-prefilters-before-semantic-model-calls

Conversation

@dnyw4l3n13

Copy link
Copy Markdown
Collaborator

Summary

Add cheap syntactic prefilters immediately before semantic-model calls (GetTypeInfo, GetSymbolInfo, FindInvokedMemberSymbol) in ten affected analyzers. Each prefilter uses Identifier.Text on the syntax node's name token and compares against a static HashSet<string>/FrozenSet<string>, avoiding expensive Roslyn semantic lookups for nodes that cannot possibly match.

Affected analyzers:

  • ProhibitedMethodsDiagnosticsAnalyzer — prefilter on banned member names before GetTypeInfo
  • ForceMethodParametersInvocationsDiagnosticsAnalyzer — prefilter on {Serialize, SerializeAsync, Deserialize, DeserializeAsync, Received}
  • ProhibitedMethodInvocationsDiagnosticsAnalyzer — prefilter on {True, False, AddOrUpdate, GetOrAdd, Equals}
  • ProhibitedMethodWithStrictParametersInvocationDiagnosticsAnalyzer — prefilter on {Received, AddJsonFile}
  • ProhibitedClassesInTestAssembliesDiagnosticsAnalyzer — syntactic receiver-name prefilter (Console)
  • ProhibitedSubstituteForUsageInTestBaseDiagnosticsAnalyzer — check name is For before GetSymbolInfo
  • ArgumentExceptionAnalysisDiagnosticsAnalyzer — prefilter on {ArgumentException, ArgumentNullException, ArgumentOutOfRangeException}
  • TestClassFieldsAnalysisDiagnosticsAnalyzer — reorder: readonly/const check BEFORE IsDerivedFromTestBase
  • TestClassPropertyAnalysisDiagnosticsAnalyzer — reorder: accessor-mutability check BEFORE IsDerivedFromTestBase
  • TestDetection.IsTestMethod — attribute-name prefilter {Fact, Theory, FactAttribute, TheoryAttribute} before GetTypeInfo

Test plan

  • All existing tests in FunFair.CodeAnalysis.Tests pass unchanged (no new or removed diagnostics)
  • Benchmarks in FunFair.CodeAnalysis.Benchmark.Tests show reduced mean time and/or allocations vs baseline
  • dotnet build succeeds with no warnings
  • dotnet buildcheck passes

Closes #457

🤖 Generated with Claude Code

Prompt: Perf Phase 1: syntactic prefilters before semantic-model calls in all affected analyzers
@dnyw4l3n13 dnyw4l3n13 self-assigned this Jul 2, 2026
@dnyw4l3n13 dnyw4l3n13 added AI-Work Work for an AI Agent Urgent Urgent Priority labels Jul 2, 2026
@dnyw4l3n13

Copy link
Copy Markdown
Collaborator Author

Pre-commit Hook Failure — Environmental Issue

The pre-commit hook is blocking the commit of the syntactic prefilter implementation due to a missing system utility in the build container.

What the hook ran

The hook detected .cs file changes and ran buildtest, which runs dotnet test on the entire solution (including FunFair.CodeAnalysis.Benchmark.Tests). The benchmark tests use BenchmarkDotNet to run micro-benchmarks, and BenchmarkDotNet internally tries to use pgrep to manage child processes. Since pgrep (from procps) is not installed in this container, BenchmarkDotNet fails to build its auto-generated boilerplate and the benchmark test assertions fail:

Build Error: Exception!
Message: ErrorStartingProcess, pgrep, .../FunFair.CodeAnalysis.Benchmark.Tests/bin/Release/net10.0, No such file or directory

Status of the actual code

The syntactic prefilter implementation is complete and correct:

  • ✅ Build: dotnet build succeeds with 0 warnings
  • ✅ Unit tests: 448/448 pass on both net9.0 and net10.0
  • ✅ Buildcheck: dotnet buildcheck reports "No errors found"
  • ✅ Formatter: dotnet cscleanup makes no changes (already formatted)
  • ✅ CI: All required checks are green (from the last committed state)

Root cause

The build container is missing the procps package (which provides pgrep). This would block any commit of .cs files in this container, not just this PR.

To unblock

Install pgrep in the build container:

apt-get install -y procps

Or, if the CI environment has pgrep available, the code can be committed from a machine with procps installed.

The uncommitted implementation is on the local working tree of branch perf/457-syntactic-prefilters-before-semantic-model-calls.

@dnyw4l3n13 dnyw4l3n13 added the Blocked Blocked by a dependency or external factor label Jul 2, 2026
credfeto added a commit to credfeto/credfeto-orchestrator that referenced this pull request Jul 3, 2026
Adds ps/pkill/free, needed by .NET benchmark tooling (BenchmarkDotNet
process-priority/affinity handling) used in downstream repos. Confirmed
locally with `podman build --format docker` of development-tools plus a
direct check that ps/pkill/free are present and functional in the built
image.

Closes #1061

Prompt: apt-get install -y procps

can you do that in the appropriate base container

needed for funfair-tech/funfair-server-code-analysis#463
credfeto added a commit to credfeto/credfeto-orchestrator that referenced this pull request Jul 3, 2026
## Summary

- Installs `procps` (`ps`, `pkill`, `free`, etc.) in
`containers/base/development-tools/Dockerfile`, the base apt-packages
layer shared by every downstream image (node/python/full/agent).
- Needed by .NET benchmark tooling (BenchmarkDotNet
process-priority/affinity handling) used in downstream repos — see
funfair-tech/funfair-server-code-analysis#463.

Closes #1061

## Test plan

- [x] Local `podman build --format docker -t development-tools:local
containers/base/development-tools` completes successfully end-to-end
(53/53 steps), including the build-time sanity check.
- [x] Verified `ps`, `pkill`, `free` are present and functional in the
built image (`ps from procps-ng 4.0.4`).
@credfeto credfeto removed the Blocked Blocked by a dependency or external factor label Jul 5, 2026
@dnyw4l3n13

Copy link
Copy Markdown
Collaborator Author

Blocked — BenchmarkDotNet Build Timeout on QEMU Container

Update: The previous comment about a missing pgrep is no longer accurate — procps is now installed. The actual blocker is different and cannot be resolved from this container.

Root cause

The pre-commit hook runs buildtest which executes dotnet test on the entire solution, including FunFair.CodeAnalysis.Benchmark.Tests. That project uses BenchmarkDotNet, which builds an auto-generated boilerplate project for each benchmark run. BenchmarkDotNet has a hardcoded 120-second build timeout. On this container (QEMU Virtual CPU version 2.5+), the compilation of that boilerplate project exceeds 120 seconds and BenchmarkDotNet aborts:

// start dotnet  build ... in FunFair.CodeAnalysis.Benchmark.Tests-DefaultJob-1
// command took longer than the timeout: 120s. Killing the process tree!
// BenchmarkDotNet has failed to build the auto-generated boilerplate code.

Why CI passes but this container fails

CI runs on [self-hosted, linux, build] runners (real hardware). The BenchmarkDotNet boilerplate build completes within 120 seconds there. On this QEMU-emulated CPU it does not.

What's blocked

  • The staged implementation is complete and correct: 10 analyzer files with syntactic prefilters
  • dotnet build succeeds with 0 warnings
  • All 448 unit tests pass (both net9.0 and net10.0)
  • CI checks are all green on the last pushed commit

To unblock

An operator needs to either:

  1. Commit and push the staged changes from a machine with a non-emulated CPU — the staged changes are ready; a build host can stage and commit the 10 files, then push, or
  2. Increase the performance of this container so the BenchmarkDotNet boilerplate build completes within 120 seconds.

The fix cannot be modifying the benchmark classes — switching them to InProcessEmitToolchain would invalidate the allocation baselines this PR was designed to establish, and is out of scope.

@dnyw4l3n13 dnyw4l3n13 added the Blocked Blocked by a dependency or external factor label Jul 5, 2026
dnyw4l3n13 added a commit to credfeto/credfeto-orchestrator that referenced this pull request Jul 6, 2026
…re once the fix ships (#1118) (#1119)

## Summary

- Fixes #1118
- Following #1115, an audit of every currently-`Blocked` PR found
`funfair-tech/funfair-server-code-analysis#463`: blocked purely because
the agent diagnosed a missing `pgrep`/`procps` in the container image.
The fix merged and propagated through the image build chain almost
immediately, but the PR just sat `Blocked` until a human happened to
notice — a manual-intervention condition this repo's own Headless
Operation Principle says must be fixed in code.
- `oneshot` gains `pr_json_env_block_image_sha` (parses a structured
`<!-- orchestrator:env-block image-sha=<sha> --> ` trailer out of the
PR's already-fetched comments — no new GitHub API call) and
`current_agent_image_sha` (`podman inspect` on the freshly-pulled
`:latest` tag's baked `IMAGE_SHA_DEVELOPMENT_AGENT` env var — no
container run needed).
- `try_auto_unblock_env_diagnosed_pr`, wired into both PR
observe-blocked branches, auto-clears `Blocked` and comments the moment
the recorded SHA differs from the current image's SHA.
- Bounded by `MAX_PR_ENV_AUTO_UNBLOCKS` (default 3): if the same failure
recurs after every rebuild, the "environment" diagnosis was probably
wrong — auto-clearing stops and a one-time comment asks a human to look,
instead of looping silently forever.
- Scope is deliberately narrow (confirmed before implementing): only
applies to blocks the agent itself tagged as environment/infrastructure.
Runaway-cap blocks (#116/#387's case), reviewer-question blocks (#321's
case), and CI-timeout/idle-exhausted blocks are untouched.
- The agent-side marker convention (`agent-roles.instructions.md` §
"Environment/Infrastructure Block Marker") is added separately in
`cs-template`, since `ai/global/` is template-managed and can't be
edited directly in this repo.

## How Has This Been Tested

- [x] `bats test/oneshot.bats` — 522/522 passing, including new unit
tests for `pr_json_env_block_image_sha`, `current_agent_image_sha`,
`try_auto_unblock_env_diagnosed_pr`, `reset_env_unblock_attempts`, and 3
`main()` integration tests (auto-clears on a newer image, stays blocked
when unchanged, stops at the cap and notifies once).
- [x] `shellcheck oneshot` — clean.
- [x] `markdownlint ai/local/debugging.instructions.md` — clean.
- [x] `dotnet buildcheck -solution src/Credfeto.Template.slnx` — no
errors (unaffected by this change).

## Types of changes

- [x] New feature (non-breaking change which adds functionality)
- [x] Additional Unit Tests/Integration Tests
- [x] Docs change (debugging instructions)

## Checklist

- [x] I have added tests to cover my changes.
- [x] Unreleased section of CHANGELOG.md has been updated with details
of this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Work Work for an AI Agent Blocked Blocked by a dependency or external factor Urgent Urgent Priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Perf Phase 1: syntactic prefilters before semantic-model calls

2 participants