AArnott / Nerdbank.MessagePack

A .NET MessagePack serialization library with great performance and simplicity.
https://aarnott.github.io/Nerdbank.MessagePack/
MIT License
33 stars 0 forks source link

Ignore members with default values #89

Open AArnott opened 14 hours ago

AArnott commented 14 hours ago

The second most voted feature request on MessagePack-CSharp requests for reducing serialized payloads by omitting properties with default values.

This most readily makes sense when serializing objects as maps, where the property name can be dropped along with the 0/nil value which itself is just one byte. When the object is serialized as an array of values, serialization payload savings are only feasible if they happen to all be at the end of the array such that the array can be truncated.

Default values could potentially be different than default(TPropertyType), since the object constructor can initialize these fields to other values. Respecting [DefaultValueAttribute(value)] seems like a no-brainer.
Another option would be a property naming pattern where property X does not serialize if property ShouldXSerialize returns false, which gives ultimate runtime support for controlling what gets serialized.

CC: @eiriktsarpalis

AArnott commented 10 hours ago

@eiriktsarpalis One of your test cases in the exhaustive set fail when I turn this setting on, because it sets a bunch of default values explicitly. And no [DefaultValue] attribute exists either. As a result, I don't serialize values that are default(TPropertyType), which ends up allowing the default constructor parameter value apply, leading to a change in value. Any suggestions as to how I can determine what to consider the default value for at least this cases?

public record RecordWithSpecialValueDefaultParams(double d1 = double.PositiveInfinity, double d2 = double.NegativeInfinity, double d3 = double.NaN, double? dn1 = double.PositiveInfinity, double? dn2 = double.NegativeInfinity, double? dn3 = double.NaN, float f1 = float.PositiveInfinity, float f2 = float.NegativeInfinity, float f3 = float.NaN, float? fn1 = float.PositiveInfinity, float? fn2 = float.NegativeInfinity, float? fn3 = float.NaN, string s = "\"\ud83d\ude00葛\ud83c\udc04\r\n\ud83e\udd2f\ud801\udc00\ud801\udc28\"", char c = '\'') : IShapeable<RecordWithSpecialValueDefaultParams>

It looks like your source generated ArgumentStateConstructorFunc retains the default values, so if I created that struct and knew how to read those values (as opposed to just setting them, which is all I do at present), then that would help. But it seems it would only help the primary constructor/init property case, while ordinary get/set properties can also have explicit defaults.

eiriktsarpalis commented 5 hours ago

I'm not sure I entirely follow the requirement, but is it that you need getters exposed on the argument state type? What about comparing against IPropertyShape.DefaultValue before calling the setter?

AArnott commented 2 hours ago

What about comparing against IPropertyShape.DefaultValue before calling the setter?

That would be awesome. Except there is no such property. You link to IConstructorParameterShape, which does have that property. I hadn't noticed that and I can and should use that. But for the properties that are not on the constructor (and/or types with a default constructor and no init properties), I'll still need the default values there. e.g.

[GenerateShape]
partial class A
{
   public int Age { get; set; } = 18;
   public int Field = 15;
}
AArnott commented 2 hours ago

Now, default parameters are limited to values that C# can express as constants, whereas property and field initializers are allowed to be any runtime-constructible value. However, we know that these initializers are limited in that they cannot access this, so they are essentially statically executable. So theoretically your source generator could lift those initializers out of context to run them within a shape delegate. It could get very tricky though, because you'd have to keep every binding the same, and C# context would change so you might actually have to change the syntax around.

A simpler scope for the fix would be to start with only supporting literal expressions in field and property initializers. But that's more limited even than 'any constant expression' that default parameters can specify...

AArnott commented 52 minutes ago

Although, I guess field and property initializers not being supported and instead asking users to apply a DefaultValueAttribute would match their previous experience with reflection-based serialization. So combining that with your constructor default parameter feature may be sufficient.