rpgmaker / NetJSON

Faster than Any Binary? Benchmark: http://theburningmonk.com/2014/08/json-serializers-benchmarks-updated-2/
MIT License
231 stars 29 forks source link

mysterious deserialization result for auto property initializer #229

Closed cubed-it closed 4 years ago

cubed-it commented 4 years ago

The following linqpad example...

<Query Kind="Program">
  <NuGetReference>NetJSON</NuGetReference>
  <Namespace>NetJSON</Namespace>
</Query>

void Main()
{
    var myObject1 = new SimpleObject1() { ID = 0, Name = "Test", Value = "Value" };
    var json1 = NetJSON.NetJSON.Serialize(myObject1, new NetJSONSettings() {SkipDefaultValue = false});
    json1.Dump();
    var recreatedObject1 = NetJSON.NetJSON.Deserialize<SimpleObject1>(json1);
    recreatedObject1.Dump();

    var myObject2 = new SimpleObject2() { ID = 0, Name = "Test", Value = "Value" };
    var json2 = NetJSON.NetJSON.Serialize(myObject2, new NetJSONSettings() {SkipDefaultValue = false});
    json2.Dump();
    var recreatedObject2 = NetJSON.NetJSON.Deserialize<SimpleObject2>(json2, new NetJSONSettings() {SkipDefaultValue = false});
    recreatedObject2.Dump();

    var myObject3 = new SimpleObject3() { ID = 0, Name = "Test", Value = "Value" };
    var json3 = NetJSON.NetJSON.Serialize(myObject3);
    json3.Dump();
    var recreatedObject3 = NetJSON.NetJSON.Deserialize<SimpleObject3>(json3);
    recreatedObject3.Dump();
}

public class SimpleObject1
{
    public int ID { get; set; }
    public string Name { get; set; }
    public string Value { get; set; }
}

public class SimpleObject2
{
    public int ID { get; set; } = -5;
    public string Name { get; set; }
    public string Value { get; set; }
}

public class SimpleObject3
{
    public SimpleObject3()
    {
        ID = 5;
    }

    public int ID { get; set; }
    public string Name { get; set; }
    public string Value { get; set; }
}

...produces the following result. SimpleObject2.Id should actually be 0, but the result is -5?

image

wmjordan commented 4 years ago

It is really weird.

rpgmaker commented 4 years ago

that is really weird indeed. What version are you using and what .net framework is this?

Thanks,

wmjordan commented 4 years ago

I checked the code with the latest release obtained from NuGet and ran the code with .NET Framework 4.5, if we set the ID to another value, like the following:

new SimpleObject2() { ID = 1, Name = "Test", Value = "Value" }

It was deserialized properly.

It seems that there is something wrong when deserializing default values, since ID=0 happens to be the default value.

wmjordan commented 4 years ago

@rpgmaker Look at this place: https://github.com/rpgmaker/NetJSON/blob/master/NetJSON/NetJSON.cs#L4472

It may be the problem.

It loads the default value of the member and compare it against the deserialized value, if they equal each other, goto propNullLabel. Thus the value 0 was not set and the default -5 was remained there.

wmjordan commented 4 years ago

I tried to use GenerateTypesInto to save the assembly, however, I got this: System.TypeLoadException: Access is denied: 'NetJSON.NetJSONSerializer`1[ConsoleApp1.SimpleObject2]'.

rpgmaker commented 4 years ago

Use GenerateAssembly flag instead

wmjordan commented 4 years ago

By analyzing the generated assembly, I confirmed the problem was caused by the aforementioned comparison against the default value of the member type. Why there was such kind of comparison? It seemed to be unnecessary and removable.

rpgmaker commented 4 years ago

The comparison is only generated if the skip default is enabled which is by default. What happens if you set it to false?

wmjordan commented 4 years ago

It had already been false in the posted code. Before calling the Deserialize method, I could verify that value was still false in the settings instance.

rpgmaker commented 4 years ago

Sorry I missed that 😔. Will try to change the code to avoid the compare and see what happens.

rpgmaker commented 4 years ago

@cubed-it, Will try to work on a solution this weekend. Thanks.

cubed-it commented 4 years ago

@cubed-it, Will try to work on a solution this weekend. Thanks.

hey, i've been following your conversation in silence ;) thanks a lot in advance!

rpgmaker commented 4 years ago

Ended up generated the code as the following

image

rpgmaker commented 4 years ago

@cubed-it, can you pull the latest from master and test your code again? I also added a unit test for your scenario which is passing for me at the moment.

I will push to nuget soon if you rather wait for that.

rpgmaker commented 4 years ago

@cubed-it , i just published to nuget a moment ago - https://www.nuget.org/packages/NetJSON/

cubed-it commented 4 years ago

my test looks good, thanks for the quick fix!

wmjordan commented 4 years ago

Ended up generated the code as the following

image

The code logic could be optimized to the following, which may save some comparisons:

if (P_4.SkipDefaultValue == false || n != 0) {
   P_2.ID = num;
}
rpgmaker commented 4 years ago

I can flip the condition and check if skip default first on my next code change. Thanks