Skip to content

Fix perf regression in testing for RPC marshalable objects#1330

Merged
AArnott merged 3 commits intomainfrom
dev/andarno/betterCaching-main
Oct 21, 2025
Merged

Fix perf regression in testing for RPC marshalable objects#1330
AArnott merged 3 commits intomainfrom
dev/andarno/betterCaching-main

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 PolyType milestone Oct 21, 2025
@AArnott AArnott enabled auto-merge October 21, 2025 02:05
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 addresses a performance regression in RPC marshalable object testing by restoring early exit behavior when a type is not an interface and introducing caching for redundant attribute checks. The changes optimize the type checking logic to reduce unnecessary attribute lookups.

Key Changes:

  • Modified TryGetMarshalOptionsForTypeHelper to return bool? instead of bool, enabling distinction between "definitely not marshalable" (false) and "unknown, needs more checking" (null)
  • Added caching for RpcMarshalableAttribute lookups via new RpcMarshalableAttributeCache dictionary
  • Replaced direct GetCustomAttribute calls with cached TryGetRpcMarshalableAttribute method

}

return false;
// Unknown. Caller may want to do more work.
Copy link

Copilot AI Oct 21, 2025

Choose a reason for hiding this comment

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

[nitpick] While more descriptive than the previous comment, this could be clearer. Consider: 'Return null to indicate the interface type requires further validation by the caller.'

Suggested change
// Unknown. Caller may want to do more work.
// Return null to indicate the interface type requires further validation by the caller.

Copilot uses AI. Check for mistakes.
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