Serialization performance improvements by steveharter · Pull Request #41098 · dotnet/corefx

steveharter · 2019-09-13T19:39:21Z

Serialization improvements from 15%-28% on existing benchmarks (for simple objects).

Changes in order of most impact to lesser impact:

Use [AggressiveInlining] when processing property values. Required some misc refactoring so that the methods inlined are only called once (instead of twice).
Using an array instead of Dictionay enumerator to obtain properties to serialize.
Changing a property to a field (struct for JsonEncodedTest) to avoid a copy on read.
Removing unused return values and misc cleanup.

WriteJson benchmarks

Faster	base/diff	Base Median (ns)	Diff Median (ns)
System.Text.Json.Serialization.Tests.WriteJson.SerializeToUtf8Bytes	1.41	1328.46	944.13
System.Text.Json.Serialization.Tests.WriteJson.SerializeToString	1.40	592.57	422.00
System.Text.Json.Serialization.Tests.WriteJson.SerializeToUtf8By	1.40	547.15	389.85
System.Text.Json.Serialization.Tests.WriteJson.SerializeToString	1.36	1372.30	1006.11
System.Text.Json.Serialization.Tests.WriteJson.SerializeToStream	1.36	653.96	480.28
System.Text.Json.Serialization.Tests.WriteJson.SerializeToStream	1.35	1435.65	1061.99
System.Text.Json.Serialization.Tests.WriteJson.SerializeToUtf8By	1.29	35629.31	27513.08
System.Text.Json.Serialization.Tests.WriteJson.SerializeToStream	1.27	34489.63	27073.81
System.Text.Json.Serialization.Tests.WriteJson.SerializeToString	1.26	36860.43	29178.57
System.Text.Json.Serialization.Tests.WriteJson.Serializ	1.20	614502.43	513175.55
System.Text.Json.Serialization.Tests.WriteJson.Serializ	1.19	618552.11	518655.83
System.Text.Json.Serialization.Tests.WriteJson.Serializ	1.19	663942.62	559237.31

Comparisons to Json.NET

MyEventsListerViewModel Before

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
JSON.NET	782.3 us	2.066 us	1.933 us	781.6 us	779.2 us	785.3 us	94.6372	47.3186	47.3186	565.39 KB
SystemTextJson	664.6 us	4.227 us	3.954 us	664.6 us	658.5 us	673.5 us	45.0928	45.0928	45.0928	381.33 KB

MyEventsListerViewModel After

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
JSON.NET	787.8 us	2.807 us	2.488 us	786.9 us	784.9 us	793.4 us	94.9367	47.4684	47.4684	564.91 KB
SystemTextJson	574.4 us	13.518 us	15.567 us	568.1 us	558.8 us	608.1 us	45.9770	45.9770	45.9770	380.87 KB

IndexViewModel Before

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
JSON.NET	40.50 us	0.1701 us	0.1591 us	40.48 us	40.26 us	40.84 us	9.6556	1.6093	-	59.33 KB
SystemTextJson	36.95 us	0.1801 us	0.1597 us	36.90 us	36.72 us	37.32 us	3.9677	0.2939	-	25.13 KB

IndexViewModel After

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
JSON.NET	40.03 us	0.1336 us	0.1249 us	40.00 us	39.88 us	40.27 us	9.5602	1.5934	-	59.33 KB
SystemTextJson	27.48 us	0.1451 us	0.1211 us	27.46 us	27.34 us	27.79 us	3.9739	0.3312	-	25.01 KB

Location Before

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
JSON.NET	1.439 us	0.0032 us	0.0028 us	1.439 us	1.434 us	1.443 us	0.2758	-	-	1736 B
SystemTextJson	1.370 us	0.0038 us	0.0034 us	1.370 us	1.366 us	1.375 us	0.0930	-	-	584 B

Location After

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
JSON.NET	1.429 us	0.0077 us	0.0069 us	1.427 us	1.422 us	1.445 us	0.2724	-	-	1736 B
SystemTextJson	1.007 us	0.0029 us	0.0026 us	1.006 us	1.002 us	1.011 us	0.0924	-	-	584 B

LoginViewModel Before

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
JSON.NET	696.4 ns	3.961 ns	3.511 ns	695.3 ns	692.5 ns	703.4 ns	0.2375	-	-	1504 B
SystemTextJson	599.8 ns	1.602 ns	1.420 ns	599.6 ns	597.6 ns	603.2 ns	0.0527	-	-	344 B

LoginViewModel After

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0/1k Op	Gen 1/1k Op	Gen 2/1k Op	Allocated Memory/Op
JSON.NET	665.9 ns	3.070 ns	2.722 ns	665.3 ns	661.1 ns	670.1 ns	0.2391	-	-	1504 B
SystemTextJson	434.6 ns	1.487 ns	1.318 ns	434.6 ns	432.6 ns	436.6 ns	0.0539	-	-	344 B

steveharter · 2019-09-13T19:40:07Z

src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Helpers.cs


-            string result;
-
-            using (var output = new PooledByteBufferWriter(options.DefaultBufferSize))


The return value was not used, so this code removed.

So, this was just pure overhead that was redundant/unnecessary work?

ahsonkhan · 2019-09-14T02:51:52Z

src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs

        private const int PropertyNameCountCacheThreshold = 64;

-        // All of the serializable properties on a POCO keyed on property name.
+        // All of the serializable properties on a POCO (except the optional extension property) keyed on property name.


By extension, do you mean JsonExtensionData?

ahsonkhan · 2019-09-14T02:52:31Z

src/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs

+                            cacheArray = new JsonPropertyInfo[cache.Count + 1];
+
+                            // Set the last element to the extension property.
+                            cacheArray[cache.Count] = DataExtensionProperty;


Why do we do this?

It avoids an extra call to HandleObject() which is now inlined (so we only have one call now, instead of two).

FYI the dictionary-based cache doesn't (and can't) contain the extension property since it is used during deserialization to match property names which we don't want to do (it is not possible to deserialize directly into the extension property - only indirectly through a missing property)

ahsonkhan · 2019-09-14T02:56:24Z

src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.cs

                            current.JsonPropertyInfo.Write(ref state, writer);
                            finishedSerializing = true;
                            break;
-                        case ClassType.Object:


Is ClassType.Object ever used? And was this code always unreachable?

Yes it is used -- if type of a property is System.Object then all properties start out with ClassType.Object and then are converted at run-time to the instance's type (via obj.GetType()). So at this point in the code, ClassType.Object is only used when the instance is System.Object (an edge case for sure).

ahsonkhan · 2019-09-14T02:58:39Z

src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.cs


                    if (finishedSerializing)
                    {
-                        if (writer.CurrentDepth == 0 || writer.CurrentDepth == originalWriterDepth)


Is this a valid change? What happens if the writer is at depth X, and then the serializer writes a bunch of things that puts the writer at depth 0? Is that scenario possible?

The serializer should never go below the original which is determined from the serializer's entry point (e.g. Serialize()). Currently originalWriterDepth is 0 for the normal case and X for re-entry cases when a writer is passed into the Serialize() method.

ahsonkhan · 2019-09-14T02:59:53Z

src/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs

+                int len = JsonClassInfo.PropertyCacheArray.Length;
+                if (PropertyEnumeratorIndex < len)
                {
+                    if ((PropertyEnumeratorIndex == len - 1) && JsonClassInfo.DataExtensionProperty != null)


How's the code coverage? Are these branches covered?

This is hit 100% of the time when we can an extension property.

To clarify, I was talking about test coverage. It looks like this change introduced test gaps that we should fix. Can you please add tests so that all new branches/lines of code being introduced in this PR are covered?

For example, NextProperty is no longer 100% covered:

Thanks. I will send a new PR to clean this up.

jozkee · 2019-09-23T21:12:44Z

src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.HandleObject.cs

@@ -72,11 +67,13 @@ private static bool WriteEndObject(ref WriteStack state)
            return true;


Code path in line 62 is not really needed, it is only hit when root object finishes and just sets root's WriteStackFrame properties to null/false, which doesn't make any difference at the end of serialization.
@steveharter

Yes thanks. It is not needed - any future push() from a subsequent serialization will reset the state.

Commit migrated from dotnet/corefx@017a038

Serialization performance improvements

03250df

steveharter added tenet-performance Performance related issue area-System.Text.Json labels Sep 13, 2019

steveharter added this to the 5.0 milestone Sep 13, 2019

steveharter requested review from ahsonkhan and layomia September 13, 2019 19:39

steveharter self-assigned this Sep 13, 2019

steveharter commented Sep 13, 2019

View reviewed changes

ahsonkhan reviewed Sep 14, 2019

View reviewed changes

layomia approved these changes Sep 16, 2019

View reviewed changes

steveharter merged commit 017a038 into dotnet:master Sep 16, 2019

steveharter deleted the SerPerf branch September 16, 2019 16:20

jozkee reviewed Sep 23, 2019

View reviewed changes

steveharter mentioned this pull request Sep 27, 2019

Address misc feedback and issues from recent perf changes #41414

Merged

steveharter added a commit to steveharter/dotnet_corefx that referenced this pull request Oct 14, 2019

Serialization performance improvements (dotnet#41098)

e94ad56

steveharter mentioned this pull request Oct 14, 2019

Port perf changes and issues #41771

Merged

steveharter added a commit to steveharter/dotnet_corefx that referenced this pull request Oct 14, 2019

Serialization performance improvements (dotnet#41098)

7673f84

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Serialization performance improvements (dotnet/corefx#41098)

4a5da0e

Commit migrated from dotnet/corefx@017a038


		string result;

		using (var output = new PooledByteBufferWriter(options.DefaultBufferSize))

		@@ -72,11 +67,13 @@ private static bool WriteEndObject(ref WriteStack state)
		return true;

Conversation

steveharter commented Sep 13, 2019

Uh oh!

steveharter Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jozkee Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

steveharter Sep 13, 2019 •

edited

Loading

jozkee Sep 23, 2019 •

edited

Loading