Remove unneeded GetTypeInfo() calls#2206
Conversation
Improve performcance by removing unneeded GetTypeInfo() calls
|
@dotnet-policy-service agree |
|
We can remove |
|
@AArnott, can you please take a look? |
AArnott
left a comment
There was a problem hiding this comment.
Thanks for this.
(a sort of breaking change, though it's in internal namespace)
We need to be very careful. Public APIs in an 'internal' namespace are likely called by source generated code and thus outside of our control and likely to break if the library changes them.
I do think one method should be reverted. But if you can prove me wrong with sharplab.io release build JIT output, I'll accept what you have here.
Sorry, what method are you talking about? I'm ready to revert it if can cause any breaking changes. |
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes performance by removing unnecessary GetTypeInfo() calls throughout the MessagePack codebase. The optimization leverages the fact that many properties that previously required TypeInfo can now be accessed directly on Type objects.
- Replaces
type.GetTypeInfo()calls with direct Type property access - Removes intermediate TypeInfo variables where no longer needed
- Adds new overloaded extension methods to support Type instead of TypeInfo
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| SkipClrVisibilityChecks.cs | Updates method parameter and all usages from TypeInfo to Type |
| ImmutableCollectionResolver.cs | Removes TypeInfo usage and adds Type extension method for IsNullable |
| DynamicUnionResolver.cs | Eliminates TypeInfo variables and uses Type properties directly |
| DynamicObjectResolver.cs | Comprehensive removal of TypeInfo usage, updates method calls and property access |
| DynamicGenericResolver.cs | Removes TypeInfo variables and converts interface checks to use Type directly |
| DynamicEnumResolver.cs | Simplifies type checking by using Type instead of TypeInfo |
| DynamicEnumAsStringResolver.cs | Updates nullable and enum type checking to use Type directly |
| DynamicEnumAsStringIgnoreCaseResolver.cs | Similar TypeInfo to Type conversion for enum handling |
| AttributeFormatterResolver.cs | Direct Type usage for custom attribute retrieval |
| MessagePackSerializer.NonGeneric.cs | Updates value type checking in expression building |
| MessagePackSecurity.cs | Refactors equality comparer selection to use Type directly |
| Sequence`1.cs | Direct Type property access for primitive type checking |
| ReflectionExtensions.cs | Adds Type overloads for existing TypeInfo extension methods |
| ILGeneratorExtensions.cs | Updates IL generation code to use Type properties |
| DynamicAssemblyFactory.cs | Updates visibility check method call to use Type |
| TypelessFormatter.cs | Removes unnecessary TypeInfo variables in serialization logic |
| PrimitiveObjectFormatter.cs | Adds Type overload for IsSupportedType method |
| EnumAsStringFormatter`1.cs | Uses Type variable to reduce repeated typeof() calls |
| DynamicObjectTypeFallbackFormatter.cs | Eliminates TypeInfo usage in fallback formatting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
AArnott
left a comment
There was a problem hiding this comment.
Looks good. Just a few redundant methods to consolidate and it will be good.
Improve performance by removing unneeded
GetTypeInfo()callsBefore:
After: