Skip to content

Conversation

StefH
Copy link
Collaborator

@StefH StefH commented May 9, 2025

No description provided.

@StefH StefH requested a review from Copilot May 9, 2025 17:26
@StefH StefH self-assigned this May 9, 2025
@StefH StefH added the bug label May 9, 2025
@StefH StefH changed the title Update DynamicGetMemberBinder to only add BindingRestrictions for dyn… Update DynamicGetMemberBinder to only add BindingRestrictions for dynamic type and cache the DynamicMetaObject May 9, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the dynamic member binding behavior to improve performance by only adding BindingRestrictions for dynamic types and caching the resulting DynamicMetaObject. It also adds a new console performance test project to benchmark these changes.

  • Added a caching mechanism using ConcurrentDictionary in DynamicGetMemberBinder.
  • Updated FallbackGetMember to conditionally add type restrictions for dynamic objects.
  • Introduced a new performance test project (ConsoleAppPerformanceTest) to measure the impact.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/System.Linq.Dynamic.Core/DynamicGetMemberBinder.cs Implements caching of DynamicMetaObject and conditionally adds type restrictions.
src-console/ConsoleAppPerformanceTest/Program.cs Introduces performance tests for dynamic binding queries.
src-console/ConsoleAppPerformanceTest/ConsoleAppPerformanceTest.csproj New project file set for performance tests using .NET Framework 4.8 and C#12.
System.Linq.Dynamic.Core.sln Updates solution to include the new ConsoleAppPerformanceTest project.
Comments suppressed due to low confidence (1)

src-console/ConsoleAppPerformanceTest/Program.cs:23

  • [nitpick] The alias 'Idee' in the dynamic projection might be a typographical error. If this naming is intentional, consider adding a comment to clarify its purpose.
_ = query.AsQueryable().Select("new { it.ID as Idee } ").ToDynamicList();

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the DynamicGetMemberBinder to improve performance by caching DynamicMetaObjects for dynamic types and by adding BindingRestrictions only for these types. Key changes include:

  • Introducing a ConcurrentDictionary cache for dynamic meta objects.
  • Changing the constructor visibility to internal.
  • Adding a dedicated performance test project to measure improvements.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/System.Linq.Dynamic.Core/DynamicGetMemberBinder.cs Added caching of DynamicMetaObjects and updated constructor visibility
src-console/ConsoleAppPerformanceTest/Program.cs Added performance testing for dynamic queries
src-console/ConsoleAppPerformanceTest/ConsoleAppPerformanceTest.csproj Configured project settings for .NET Framework and C# 12
System.Linq.Dynamic.Core.sln Updated solution file to include the new test project
Comments suppressed due to low confidence (1)

src/System.Linq.Dynamic.Core/DynamicGetMemberBinder.cs:20

  • Ensure that changing the constructor visibility from public to internal aligns with the intended usage of DynamicGetMemberBinder and that any dependent external code is updated accordingly or documented.
internal DynamicGetMemberBinder(string name, ParsingConfig? config) : base(name, config?.IsCaseSensitive != true)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

1 participant