Skip to content

Fix perf regression in testing for RPC marshalable objects#1329

Merged
AArnott merged 2 commits intov2.23from
dev/andarno/betterCaching
Oct 21, 2025
Merged

Fix perf regression in testing for RPC marshalable objects#1329
AArnott merged 2 commits intov2.23from
dev/andarno/betterCaching

Conversation

@AArnott
Copy link
Copy Markdown
Member

@AArnott AArnott commented Oct 21, 2025

Some recent refactoring inadvertently caused RPC marshalable object testing to not break out early when the type is not an interface.
In this change, I correct that mistake. I also cache redundant attribute checks for interface types.

As reported in devdiv-2575202

@AArnott AArnott added this to the v2.23 milestone Oct 21, 2025
Copy link
Copy Markdown

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 fixes a performance regression in RPC marshalable object testing that was introduced by recent refactoring. The key issue was that the code no longer broke out early when testing non-interface types, causing unnecessary work. The fix also adds caching for redundant attribute checks on interface types.

Key Changes:

  • Modified TryGetMarshalOptionsForTypeHelper to return bool? instead of bool to distinguish between "definitely not marshalable" (false), "definitely marshalable" (true), and "unknown, needs more checking" (null)
  • Added RpcMarshalableAttributeCache to cache RpcMarshalableAttribute lookups and reduce redundant reflection calls
  • Introduced TryGetRpcMarshalableAttribute helper method to centralize and cache attribute retrieval

@AArnott AArnott enabled auto-merge October 21, 2025 02:01
@AArnott
Copy link
Copy Markdown
Member Author

AArnott commented Oct 21, 2025

This is being validated for effectiveness with this validation insertion.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants