Add polymorphism support to System.Text.Json #53882
Conversation
eiriktsarpalis
added
NO-MERGE
|
Tagging subscribers to this area: @eiriktsarpalis, @layomia Issue DetailsThis is a draft PR showcasing the prototype implementation for polymorphic serialization support in System.Text.Json (#45189). No APIs have been approved for this feature yet. Please see this PR for an overview of the proposed design. OverviewConceptually, the implementation can be broken into two distinct sets of changes:
Infrastructural ChangesHere is a high-level overview of the infrastructural changes being made:
Functional ChangesPlease see the design document for a high-level overview of the proposed features.
|
|
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. |
|
Not sure if it's the right place to address this, but I have a question about the type discriminator, not about the lax/strict mode but about the way it is expressed: has anybody considered using a 2-cell array |
Why would this be preferable? If anything it might make more difficult to evaluate what |
|
This is a bit more compact, avoids name clashes, and refs and ids can also be wrapped into such 2-cells arrays. Anther aspect that this introduces is that the data shape is actually different whether you know the type or not. This (up to me) provides a "kind of type check": a datum that is polymorphic must be read by handling the cell wrapper, it cannot be (wrongly) confused with a typed object (that is not expected), by silently skipping the $type discriminator property. I perfectly understand that you'd go the "classical" way but I think this approach is (more) interesting. (And yes, I know that others do this but - imho - this is not a fundamental argument.) |
That's not true, collection types can also be polymorphic (following the |
|
Absolutely. In the "classical way", polymorphic collections are already structurally different. |
|
Just a simple example with:
An array of Student (the item's type is final - no ambiguities) would be An array of Person would be The same array of Persons except that it is known only as an (The type name "A(Person)" is just an example. I just use ( instead of < because of JSON \u(nicode handling).) Dictionaries ("M(string,Person)"), Sets, List etc. are of course as easily mapped. |
|
Another interesting point that I'd like to higlight: with this schema, the de/serialization of the type itself is exactly the same, it's up to the container/referencer/caller to wrap (or not) the type. Even the type name is not necessarily known/required by the de/serialization code. This allows a clean kind of "onion handling" in code: null(type(data))... (I don't sell anything and I don't want to waste your time: if you're interested feel free to continue this thread or contact me.) |
eiriktsarpalis
force-pushed
the
polymorphic-serialization
branch
from
e2f9976 to
9a2b86d
Compare
|
My last try (promised!) - This is a doc I wrote for my implementation (at least I would have tried everything I could to gain attention on this)... Json polymorphism supportOne of the big issue to solve when de/serializing objects is to manage polymorphism: when an abstraction must be serialized, the type of the serialized object must also be written so that deserialization know what to instantiate. The classical approachThe classical approach here is to inject a Note that, in this approach, serialized objects with or without a type are structurally identical (only the existence of the
The We consider "Structural Difference" between unambiguous and polymorphic objects to be a good thing since this obliges the reader to take care of the object's type if there is one. Structural PolymorphismWe are using a different approach that offers the following advantages:
Thanks to this, the code is simple and can be split in 3 layers:
Last (but not least) advantage of this approach: when deserializing, the type is known BEFORE the object's data. This enable deserializer to easily work in "pure streaming" (no lookup required, no intermediate instantiation): the Utf8Reader can be used directly. Below is a simple example with: class Person
{
public string Name { get; set; }
}
class Student : Person
{
public int Age { get; set; }
}
class Teacher : Person
{
public bool IsChief { get; set; }
}The C# The C# When the same array of Persons is known only as an object[] (or any other non-unique "base" type like the non-generic |
eiriktsarpalis
force-pushed
the
polymorphic-serialization
branch
2 times, most recently
from
00882ba to
768acab
Compare
eiriktsarpalis
force-pushed
the
polymorphic-serialization
branch
from
768acab to
8a0d317
Compare
|
I would like to ask if mirroring |
|
Closing this draft since the two features are to be reviewed and implemented independently. |
ghost
locked as resolved and limited conversation to collaborators
This is a draft PR showcasing the prototype implementation for polymorphic serialization support in System.Text.Json (#45189). No APIs have been approved for this feature yet. Please see this PR for an overview of the proposed design.
Overview
Conceptually, the implementation can be broken into two distinct sets of changes:
Infrastructural Changes
Here is a high-level overview of the infrastructural changes being made:
CanBePolymorphicandCanUseDirectReadOrWriteproperties fromJsonConverterhave been moved toJsonTypeInfo, since the value of these flags can now be influenced by user configuration or attributes.WriteStack/ReadStackstructs to accommodate root-level polymorphic objects: the existing implementation depends on root-level values not being polymorphic, however this invariant can no longer be honored.JsonConverter.RuntimeTypeproperty: its only purpose is to construct a defaultConstructorDelegateinstance for interface converters, however it is being passed around pervasively in theJsonTypeInfo/JsonPropertyInfoinfrastructure and it can result in unsound behaviour in the context of polymorphic deserialization. The changes replace the property with aConstructorDelegate?property inJsonConverter. In order to minimize churn, I have kept all otherRuntimeTypeInfomembers in our infrastructure for now, but at this point they are all guaranteed to equalTypeToConvert. The final PR should be removing these altogether.ReferenceResolver.IngoreCyclesimplementation: the current implementation contains bespoke code to avoid polymorphic converters triggering false-negative cycles. I have refactored the code so that the cycle detection logic is combined with the polymorphic serialization infrastructure.ConverterListclass generic so that it can be reused with other list types inJsonSerializerOptions.Functional Changes
Please see the design document for a high-level overview of the proposed features.
Performance
The changes have a small but measurable impact on deserialization performance, since the hot path now needs to account for polymorphism. Here are some results using the dotnet/performance benchmark suite:
ReadJson_Int32
ReadJson_SimpleStructWithProperties
ReadJson_MyEventsListerViewModel
The comparison results can fluctuate across runs, however the trend clearly seems to be pointing towards decreased performance.
On the serialization side performance tends to be on par with
main:WriteJson_Int32
WriteJson_SimpleStructWithProperties
WriteJson_MyEventsListerViewModel