Conversation
Analyzers that look for a better overload (MA0002, MA0032, MA0040, TimeProvider rules) call LookupSymbols for every invocation. The same method on the same type is looked up again and again, and each lookup scans all imported namespaces for extension methods. Cache the result per file, enclosing type, container type and method name. Imports and member accessibility can only differ between these scopes, so calls in the same scope get the same answer.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The cache key preserves the lookup context, and the added tests cover the principal scope-leak risks.
Review effort: Balanced
Findings: None
What changed in this PR
Caches OverloadFinder.LookupSymbols results to improve analyzer performance while preserving scope-sensitive behavior.
Changes:
- Adds a thread-safe lookup cache keyed by syntax tree, enclosing type, container, method name, and extension-method mode.
- Adds MA0040 tests covering namespace, file, top-level, nested-type, and accessibility boundaries.
- Validation was limited to static review; builds and tests were not run.
| File | Description |
|---|---|
src/Meziantou.Analyzer/Internals/OverloadFinder.cs |
Implements scope-aware symbol lookup caching. |
tests/Meziantou.Analyzer.Test/Rules/UseAnOverloadThatHasCancellationTokenAnalyzerTests.cs |
Tests cache isolation across relevant scopes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This looks promising! Since the cache depends on the containing symbol, its validity is tied to a specific method. Maybe we could update the analyzers to use RegisterSymbolStart or RegisterOperationStartAction, allowing us to create a local cache there. That way, we could avoid using a ConcurrentDictionary that keeps data in memory for methods already analyzed. |
The lookup cache is now an OverloadLookupCache created per named type by RegisterSymbolStartAction and passed through OverloadOptions, so it is freed with the type instead of living for the whole compilation. The key uses SymbolEqualityComparer for the container symbol. The tree and enclosing declaration stay in the key because of partial and nested types.
3.0.292 caches the overload lookups of OverloadFinder per analyzed type (meziantou/Meziantou.Analyzer#1621). On a local profile of the solution it cut Meziantou.Analyzer from 31 s to 21 s of analyzer time, most of it in the server unit tests, whose analysis shard ends the pull request run.
What
OverloadFindercan now cacheLookupSymbolsresults. The analyzers create anOverloadLookupCacheper named type (RegisterSymbolStartAction) and pass it throughOverloadOptions.LookupCache, so the cache is dropped when the type is done. Code fixers don't pass one and keep uncached lookups.Why
MA0002, MA0032, MA0040 and other rules look up overloads for every invocation. Projects call the same methods (
Task.Delay,Stream.ReadAsync, ...) many times, and every lookup scans all imported namespaces again.Meziantou.Analyzer time on a large solution (median of 5 builds): 37.0 s on main, 15.6 s with this change.
Why the key is safe
The key is the file, the enclosing type declaration, the container type (compared with
SymbolEqualityComparer) and the method name. The lookup result can only change with theusingdirectives in scope or with which members are accessible, and both are the same within one type declaration in one file. Partial types and nested types get separate entries.Tests
New MA0040 tests check that the cache does not leak between scopes:
Another test checks that invocations in top-level statements are still reported.