Runtime: Huge performance degradation using System.Text.Json to serialize a model with an Object type property

Created on 8 Oct 2019 · 9Comments · Source: dotnet/runtime

I've been troubleshooting some performance issues that came up during a core 2.2 -> 3.0 upgrade, and was stumped why a particular ASP.NET Core API endpoint showed terrible performance compared to the others.

After countless hours I finally discovered the cause; the model being serialized contained a child class, with a collection property that was simply defined with type Object, i.e. what should have been:
```c#
public IEnumerable

Tables { get; set; }

was defined as: 
```c#
public object Tables { get; set; }

The reasons for this odd coding choice are unfortunately lost in history, but it works fine, was insidiously hard to diagnose, and the perf impact was very severe, with the API going from ~300 requests/sec under local load tests to ~3300 requests/sec after fixing.

I assume that the poor performance is due to reflection being used to serialize when there's an unknown object type, and I guess it only gets worse because it's a list. It's not clear to me if there's a way of fixing this, or if it would be necessary to remove support for serializing Object altogether. Perhaps just log a warning to make it easier to catch?

I'd be happy to try my hand with a PR, if someone could point me in the right direction?

area-System.Text.Json tenet-performance

Source

mrlund

All 9 comments

Thanks for reporting this @mrlund, that looks like a severe regression which we should definitely try and fix. It isn't isolated to just collections (serializing the simple login view model test also regresses roughly 15x). Though collections (like IEnumerable<T>) degrade in perf a lot more when treated as objects.
https://github.com/dotnet/performance/blob/3d4bb67fcff40596f3deb1eea6095b12aa4fe90f/src/benchmarks/micro/Serializers/DataGenerator.cs#L206-L211

Looks like @svick is working on a fix for this: https://github.com/dotnet/corefx/pull/41753

Did you notice any similar issues with deserializing or was this perf issue unique to serialization (in my quick testing, I noticed a perf difference there but it wasn't this significant - 60%)?

ahsonkhan on 14 Oct 2019

@ahsonkhan I didn't try deserializing since my application doesn't do that for this model, and generally doesn't deserialize anything beyond simple models. If it's helpful I'd be happy to setup a case where it does, and report back?

mrlund on 14 Oct 2019

If it's helpful I'd be happy to setup a case where it does, and report back?

~That would be helpful for sure! I want to make sure that when we fix this issue, it's addressed for all cases rather than only the specific one you ran in to :)~

Edit: Actually, never mind. I just remembered that for deserializing object types, we create and return a boxed JsonElement. That is expected to be slower (at least a bit).

ahsonkhan on 14 Oct 2019

It's a bit of a contrived test, since what I did was serialize the model using Json.NET and then deserialize it again with System.Text. From my test it doesn't seem like there's any significant performance difference deserializing the model with object type property*, so the issue seems constrained to serialization.

As a point of interest it seems that Json.NET doesn't have any performance difference serializing the polymorphic type vs the concrete type.

*Since it's a real application use case, with a lot of other things involved, and I'm running it as a local load test the resulting requests/sec do vary +/- 15% from each run. It was probably just a little slower deserializing the object version, but not enough to be noticable beyond the normal variance.

mrlund on 14 Oct 2019

👍1

From my test it doesn't seem like there's any significant performance difference deserializing the model with object type property*, so the issue seems constrained to serialization.

Great, thanks for checking.

As a point of interest it seems that Json.NET doesn't have any performance difference serializing the polymorphic type vs the concrete type.

Makes sense given the implementation. I believe S.T.Json can be fixed to not vary all that much either between the two cases but that would require some investigation/validation after the caching bug is fixed (which is the primary culprit here).

ahsonkhan on 14 Oct 2019

Port https://github.com/dotnet/corefx/pull/41753 to 3.1

ahsonkhan on 21 Oct 2019

I'll create the 3.1 PR.