perf(polyfill): skip semantic binding on non-lambda invocations#42
Merged
perf(polyfill): skip semantic binding on non-lambda invocations#42
Conversation
Narrow the syntactic predicate to require at least one lambda argument before reaching for the SemanticModel. Every intercepted call site (ExpressionPolyfill.Create and the IExpressiveQueryable stubs) takes a lambda; method-group and Func<>-variable args were never intercepted. Ordinary non-lambda member-access calls (sb.Append, list.Add, etc.) now drop out at the predicate level instead of paying per-invocation GetSymbolInfo + GetTypeInfo cost on every cold build. Also consolidate the per-invocation symbol binding into a single exclusive dispatch (ExpressionPolyfill.Create vs IExpressiveQueryable), and gate the polyfill path behind a cheap method-name-token check so non-"Create" invocations never enter that branch. Add PolyfillColdBuildWithNoiseBenchmarks to exercise the case the existing benchmarks can't: files containing a mix of real call sites and ordinary non-intercept invocations. On the new bench at 100 noise invocations per file (10 files x 5 real call sites): Cold: 81.5ms -> 4.2ms (19.5x) Cold_E2E: 155.8ms -> 4.2ms (37.5x) Incremental_EditCallSiteFile: 5.4ms -> 0.46ms (11.6x) At 0 noise invocations the numbers are within measurement noise of pre-change, so the incremental-edit cost PR #41 drove down is preserved. Allocation at noise=100 drops 64% (5.5MB -> 2.0MB). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+185
to
+193
| if (ma.Name.Identifier.Text == PolyfillMethodName && | ||
| method.ContainingType?.ToDisplayString() == PolyfillTypeName) | ||
| { | ||
| methodCode = TryEmitPolyfill(inv, model, method, line, col, fileTag, spc); | ||
| } | ||
| else | ||
| { | ||
| methodCode = TryEmit(inv, ma, model, method, line, col, fileTag, spc); | ||
| } |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes PolyfillInterceptorGenerator cold-build performance by tightening the syntactic predicate so the generator only engages semantic binding for invocation sites that can actually be intercepted (i.e., invocations with at least one lambda argument). It also adds a benchmark that better represents “real code” by mixing intercepted call sites with large volumes of ordinary member-access invocations.
Changes:
- Require a syntactic lambda argument (
HasLambdaArgument) before treating an invocation as a candidate, both at the file prefilter stage and during per-invocation processing. - Bind the invocation’s
IMethodSymbolonce per surviving invocation and use exclusive dispatch (polyfill vsIExpressiveQueryable) to avoid redundant symbol binding. - Add
PolyfillColdBuildWithNoiseBenchmarksto measure cold-build and incremental-edit behavior with realistic “noise” invocations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/ExpressiveSharp.Generator/PolyfillInterceptorGenerator.cs |
Narrows candidate detection to lambda-bearing invocations and binds method symbols once for exclusive dispatch. |
benchmarks/ExpressiveSharp.Benchmarks/PolyfillGeneratorBenchmarks.cs |
Adds a new benchmark modeling mixed real call sites + non-intercept member-access noise. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
285
to
286
| /// </summary> | ||
| private static string? TryEmitPolyfill(InvocationExpressionSyntax inv, |
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.
Summary
PolyfillInterceptorGeneratorsyntactic predicate to require a lambda argument before engaging theSemanticModel. Non-lambda member-access calls (sb.Append,list.Add,Console.WriteLine, …) are filtered out before anyGetSymbolInfo/GetTypeInfo.ExpressionPolyfill.CreatevsIExpressiveQueryable), gated by a cheap method-name-token check for the polyfill path.PolyfillColdBuildWithNoiseBenchmarkscovering the case the existing benches can't: real call sites mixed with ordinary non-intercept invocations (the shape of production code).Why
PR #41 fixed incremental IDE edits via reference-equality caching on
CompilationUnitSyntax. It does not fix two other paths:On both, the pre-existing predicate fired
GetSymbolInfoon everyMemberAccessExpressionSyntaxinvocation with ≥1 arg, including tens of thousands of ordinary non-LINQ calls. Every intercepted call site the generator actually emits takes a lambda, so requiring a lambda syntactically is a safe, strong filter.Results
Measured with
PolyfillColdBuildWithNoiseBenchmarks(10 files × 5 real call sites,NoiseInvocationsPerFilevaried):ColdbaselineColdoptimizedCold_E2EbaselineCold_E2EoptimizedIncremental_EditCallSiteFilebaselineIncremental_EditCallSiteFileoptimizedMemory at noise=100: 5,538 KB → 1,965 KB (−64 %).
At noise=0 everything is within measurement noise, so PR #41's incremental-edit win is preserved intact.
Safety
query.Select(SomeMethod)) andFunc<>-variable args (query.Select(f)) were never intercepted — bothTryEmitPolyfillandEmitGenericLambdaalready returned null for them, and the stubs throwUnreachableExceptionat runtime. The tighter predicate drops these one step earlier; observable behavior is identical.EXP0013(warns on non-[Expressive]method groups) lives inExpressiveSharp.CodeFixers, an independent analyzer. Unaffected.ExpressiveSharp.IntegrationTests(228),ExpressiveSharp.EntityFrameworkCore.IntegrationTests(562). Every.verified.txtmatches byte-for-byte, so the intercepted set and emitted code are unchanged.Test plan
dotnet test tests/ExpressiveSharp.Generator.Tests— 1239 passed (net8/9/10)dotnet test tests/ExpressiveSharp.IntegrationTests— 228 passeddotnet test tests/ExpressiveSharp.EntityFrameworkCore.IntegrationTests— 562 passedPolyfillColdBuildWithNoiseBenchmarksbaseline + optimized run on the same machine, numbers abovePolyfillSingleFileBenchmarks+PolyfillMultiFileBenchmarksspot-checked — no regression🤖 Generated with Claude Code