Remove all allocations when resolving and make it even faster#189
Remove all allocations when resolving and make it even faster#189
Conversation
…dd a benchmark for constructor resolution
There was a problem hiding this comment.
Pull request overview
This PR optimizes the reflection-based fallback in ProjectionExpressionResolver by caching fully-resolved LambdaExpression instances (instead of rebuilding them per call), and extends the benchmarks to include constructor resolution so registry vs. reflection performance can be compared more directly.
Changes:
- Replace the reflection “factory delegate” cache with a
LambdaExpressioncache to eliminate repeated expression-tree construction. - Switch the reflection path to invoke the generated
Expressionmethod once and reuse the resulting expression instance. - Add a projectable copy-constructor and benchmark coverage for constructor resolution (registry and reflection paths).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs | Cache resolved reflection expressions (and a null sentinel) instead of caching compiled factories. |
| benchmarks/EntityFrameworkCore.Projectables.Benchmarks/Helpers/TestEntity.cs | Add a projectable copy constructor (and explicit parameterless ctor) for benchmarking constructor resolution. |
| benchmarks/EntityFrameworkCore.Projectables.Benchmarks/ExpressionResolverBenchmark.cs | Add registry/reflection benchmark cases for resolving the copy constructor expression. |
Comments suppressed due to low confidence (1)
src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs:311
expressionFactoryType.MakeGenericType(originalDeclaringType.GenericTypeArguments)will throw whenoriginalDeclaringTypeis a generic type definition (becauseGenericTypeArgumentsis empty). SinceFindGeneratedExpressionViaReflectionis public and documented for open-generic scenarios, consider handlingoriginalDeclaringType.IsGenericTypeDefinitionexplicitly (e.g., useGetGenericArguments()or short-circuit with a clear return/exception) before callingMakeGenericType.
if (expressionFactoryType.IsGenericTypeDefinition)
{
expressionFactoryType = expressionFactoryType.MakeGenericType(originalDeclaringType.GenericTypeArguments);
}
src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs
Outdated
Show resolved
Hide resolved
src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs
Outdated
Show resolved
Hide resolved
src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs
Outdated
Show resolved
Hide resolved
…sionResolver.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…sionResolver.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Likely yes, as the registry acts as a cold cache for many companion expressions and BuildReflectionExpression may still run many times concurrently upon startup if multiple concurrent requests hit it |
Updated [EntityFrameworkCore.Projectables](https://github.com/EFNext/EntityFrameworkCore.Projectables) from 5.0.2 to 6.0.0. <details> <summary>Release notes</summary> _Sourced from [EntityFrameworkCore.Projectables's releases](https://github.com/EFNext/EntityFrameworkCore.Projectables/releases)._ ## 6.0.0 ## What's Changed ### Major changes * Add support for projectable method overloads by @PhenX in EFNext/EntityFrameworkCore.Projectables#143 * Add C#14 extension members by @PhenX in EFNext/EntityFrameworkCore.Projectables#148 * Support explicitly implemented interface members and default interface properties by @rhodon-jargon in EFNext/EntityFrameworkCore.Projectables#135 * Support block-bodied members with [Projectable] attribute by @Copilot in EFNext/EntityFrameworkCore.Projectables#152 * Add ExpandEnumMethods to expand enum extension calls into ternary expressions by @Copilot in EFNext/EntityFrameworkCore.Projectables#150 * Add support for pattern matching in various cases by @PhenX in EFNext/EntityFrameworkCore.Projectables#158 * Add support for projectable constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#161 * AOT-compatible static projection registry + SyntaxFactory-based emission by @Copilot in EFNext/EntityFrameworkCore.Projectables#166 * Improve UseMemberBody and add new diagnostics by @PhenX in EFNext/EntityFrameworkCore.Projectables#174 * Add code fixes for EFP0001, EFP0002 and EFP0008, with tests by @PhenX in EFNext/EntityFrameworkCore.Projectables#181 * Add a code fixer and a code refactorer to transform factory methods into constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#183 * Improve source generator and analyzer responsiveness by @PhenX in EFNext/EntityFrameworkCore.Projectables#171 * Docs website by @PhenX in EFNext/EntityFrameworkCore.Projectables#194 * Implement global MSBuild defaults for [Projectable] options by @koenbeuk in EFNext/EntityFrameworkCore.Projectables#191 ### Other changes * Check source generator output compilation by @PhenX in EFNext/EntityFrameworkCore.Projectables#156 * Fix GetImplementingProperty issue when using interfaces by @PhenX in EFNext/EntityFrameworkCore.Projectables#144 * Full qualification not needed anymore by @PhenX in EFNext/EntityFrameworkCore.Projectables#157 * Split generator tests in different classes by @PhenX in EFNext/EntityFrameworkCore.Projectables#162 * Refactor ExpressionSyntaxRewriter into focused partial class files by @Copilot in EFNext/EntityFrameworkCore.Projectables#163 * Update readme and add new test for constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#165 * Remove obsolete code by @PhenX in EFNext/EntityFrameworkCore.Projectables#167 * Add source generator self-benchmark covering all expression transformers by @Copilot in EFNext/EntityFrameworkCore.Projectables#168 * Fix null conditional rewrite generating invalid member access on nullable value types by @Copilot in EFNext/EntityFrameworkCore.Projectables#169 * Improve generator benchmark with more realistic scenario by @PhenX in EFNext/EntityFrameworkCore.Projectables#170 * Fix stale incremental generator cache when referenced types change in other source files by @Copilot in EFNext/EntityFrameworkCore.Projectables#172 * Add tests for properties with a getter and setter by @PhenX in EFNext/EntityFrameworkCore.Projectables#173 * UseMemberBody more strict with expressions by @PhenX in EFNext/EntityFrameworkCore.Projectables#175 * Add custom Copilot instructions by @PhenX in EFNext/EntityFrameworkCore.Projectables#177 * Reorganize generator project by @PhenX in EFNext/EntityFrameworkCore.Projectables#178 * Add closure resolution benchmark by @PhenX in EFNext/EntityFrameworkCore.Projectables#179 * Feature/optimize closures by @PhenX in EFNext/EntityFrameworkCore.Projectables#180 * Feature/optimize resolver by @PhenX in EFNext/EntityFrameworkCore.Projectables#182 * Update github project urls by @PhenX in EFNext/EntityFrameworkCore.Projectables#184 * Update deps by @PhenX in EFNext/EntityFrameworkCore.Projectables#185 * Remove obsolete verified files and name net8 files correctly by @PhenX in EFNext/EntityFrameworkCore.Projectables#186 * Remove all allocations when resolving and make it even faster by @PhenX in EFNext/EntityFrameworkCore.Projectables#189 * Add devcontainer configuration for C# (.NET) development by @koenbeuk in EFNext/EntityFrameworkCore.Projectables#190 **Full Changelog**: EFNext/EntityFrameworkCore.Projectables@v5.0.2...v6.0.0 Commits viewable in [compare view](EFNext/EntityFrameworkCore.Projectables@v5.0.2...v6.0.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: James Gunn <james@gunn.io>
Updated [EntityFrameworkCore.Projectables](https://github.com/EFNext/EntityFrameworkCore.Projectables) from 5.0.2 to 6.0.0. <details> <summary>Release notes</summary> _Sourced from [EntityFrameworkCore.Projectables's releases](https://github.com/EFNext/EntityFrameworkCore.Projectables/releases)._ ## 6.0.0 ## What's Changed ### Major changes * Add support for projectable method overloads by @PhenX in EFNext/EntityFrameworkCore.Projectables#143 * Add C#14 extension members by @PhenX in EFNext/EntityFrameworkCore.Projectables#148 * Support explicitly implemented interface members and default interface properties by @rhodon-jargon in EFNext/EntityFrameworkCore.Projectables#135 * Support block-bodied members with [Projectable] attribute by @Copilot in EFNext/EntityFrameworkCore.Projectables#152 * Add ExpandEnumMethods to expand enum extension calls into ternary expressions by @Copilot in EFNext/EntityFrameworkCore.Projectables#150 * Add support for pattern matching in various cases by @PhenX in EFNext/EntityFrameworkCore.Projectables#158 * Add support for projectable constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#161 * AOT-compatible static projection registry + SyntaxFactory-based emission by @Copilot in EFNext/EntityFrameworkCore.Projectables#166 * Improve UseMemberBody and add new diagnostics by @PhenX in EFNext/EntityFrameworkCore.Projectables#174 * Add code fixes for EFP0001, EFP0002 and EFP0008, with tests by @PhenX in EFNext/EntityFrameworkCore.Projectables#181 * Add a code fixer and a code refactorer to transform factory methods into constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#183 * Improve source generator and analyzer responsiveness by @PhenX in EFNext/EntityFrameworkCore.Projectables#171 * Docs website by @PhenX in EFNext/EntityFrameworkCore.Projectables#194 * Implement global MSBuild defaults for [Projectable] options by @koenbeuk in EFNext/EntityFrameworkCore.Projectables#191 ### Other changes * Check source generator output compilation by @PhenX in EFNext/EntityFrameworkCore.Projectables#156 * Fix GetImplementingProperty issue when using interfaces by @PhenX in EFNext/EntityFrameworkCore.Projectables#144 * Full qualification not needed anymore by @PhenX in EFNext/EntityFrameworkCore.Projectables#157 * Split generator tests in different classes by @PhenX in EFNext/EntityFrameworkCore.Projectables#162 * Refactor ExpressionSyntaxRewriter into focused partial class files by @Copilot in EFNext/EntityFrameworkCore.Projectables#163 * Update readme and add new test for constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#165 * Remove obsolete code by @PhenX in EFNext/EntityFrameworkCore.Projectables#167 * Add source generator self-benchmark covering all expression transformers by @Copilot in EFNext/EntityFrameworkCore.Projectables#168 * Fix null conditional rewrite generating invalid member access on nullable value types by @Copilot in EFNext/EntityFrameworkCore.Projectables#169 * Improve generator benchmark with more realistic scenario by @PhenX in EFNext/EntityFrameworkCore.Projectables#170 * Fix stale incremental generator cache when referenced types change in other source files by @Copilot in EFNext/EntityFrameworkCore.Projectables#172 * Add tests for properties with a getter and setter by @PhenX in EFNext/EntityFrameworkCore.Projectables#173 * UseMemberBody more strict with expressions by @PhenX in EFNext/EntityFrameworkCore.Projectables#175 * Add custom Copilot instructions by @PhenX in EFNext/EntityFrameworkCore.Projectables#177 * Reorganize generator project by @PhenX in EFNext/EntityFrameworkCore.Projectables#178 * Add closure resolution benchmark by @PhenX in EFNext/EntityFrameworkCore.Projectables#179 * Feature/optimize closures by @PhenX in EFNext/EntityFrameworkCore.Projectables#180 * Feature/optimize resolver by @PhenX in EFNext/EntityFrameworkCore.Projectables#182 * Update github project urls by @PhenX in EFNext/EntityFrameworkCore.Projectables#184 * Update deps by @PhenX in EFNext/EntityFrameworkCore.Projectables#185 * Remove obsolete verified files and name net8 files correctly by @PhenX in EFNext/EntityFrameworkCore.Projectables#186 * Remove all allocations when resolving and make it even faster by @PhenX in EFNext/EntityFrameworkCore.Projectables#189 * Add devcontainer configuration for C# (.NET) development by @koenbeuk in EFNext/EntityFrameworkCore.Projectables#190 **Full Changelog**: EFNext/EntityFrameworkCore.Projectables@v5.0.2...v6.0.0 Commits viewable in [compare view](EFNext/EntityFrameworkCore.Projectables@v5.0.2...v6.0.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: James Gunn <james@gunn.io>
(Also add a benchmark for constructor resolution)
It is now as fast as the resgistry, so ... is the registry still needed ?