-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Support use of fast-path serialization in combined JsonSerializerContexts. #80741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support use of fast-path serialization in combined JsonSerializerContexts. #80741
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsFixes #71933 by adding a new NB the new property hasn't been approved in API review yet.
|
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
Outdated
Show resolved
Hide resolved
|
Do we actually need new API? I can see at least two ways we can do it without new API:
|
That might just work actually. We could remove the runtime invalidation altogether and just decide if the fast path should be assigned at the context layer. |
Actually, I was wrong about this. We would still need to expose new API but even then this cannot account for contexts generated by older versions of the sdk; we still need to do this invalidation in the JsonTypeInfo layer. |
...tem.Text.Json/src/System/Text/Json/Serialization/Converters/JsonMetadataServicesConverter.cs
Outdated
Show resolved
Hide resolved
|
Converting to draft, as more infrastructural changes on JsonTypeInfo are required before we are able to implement proper invalidation. |
e4a54f0 to
598634b
Compare
...System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should you be checking IsConfigured here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be false if you're configuring recursive types. Checking IsConfigured on JsonPropertyInfo is a different story, it completes regardless of cycles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this also check for Modifiers.Count == 0 and GetType() == typeof(DefaultJsonTypeInfoResolver)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case you don't need to, any customization to the particular JsonTypeInfo would get picked up by the IsModified property.
0be172d to
09f8062
Compare
Fixes #71933 by adding a new
JsonTypeInfo.OriginatingResolverpublic property that is used by source generatedJsonSerializerContextinstances to register themselves withJsonTypeInfoinstances that they generate. This information can be used to determine if the fast-path serialization delegate is compatible with the current configuration, even if the originatingJsonSerializerContextis encapsulated behind a custom resolver.