Skip to content

Comments

Modify XmlSerializators to use typed XmlWriter.WriteValue method#75415

Closed
TrayanZapryanov wants to merge 3 commits intodotnet:mainfrom
TrayanZapryanov:fix_writing_primitive_types_in_serializers
Closed

Modify XmlSerializators to use typed XmlWriter.WriteValue method#75415
TrayanZapryanov wants to merge 3 commits intodotnet:mainfrom
TrayanZapryanov:fix_writing_primitive_types_in_serializers

Conversation

@TrayanZapryanov
Copy link
Contributor

@TrayanZapryanov TrayanZapryanov commented Sep 11, 2022

Modify XmlSerializators to use typed XmlWriter.WriteValue method instead of converting to string when value is primitive.
Once Optimization goes in, this should reduce allocations.

Here are some benchmarks from performance repo:
BenchmarkDotNet=v0.13.1.1847-nightly, OS=Windows 10 (10.0.19043.1889/21H1/May2021Update)
11th Gen Intel Core i9-11900K 3.50GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK=7.0.100-preview.7.22377.5
[Host] : .NET 7.0.0 (7.0.22.37506), X64 RyuJIT AVX2
Job-LGCOMF : .NET 8.0.0 (42.42.42.42424), X64 RyuJIT AVX2

PowerPlanMode=00000000-0000-0000-0000-000000000000 Toolchain=CoreRun IterationTime=250.0000 ms
MaxIterationCount=20 MinIterationCount=15 WarmupCount=1

Benchmark : MicroBenchmarks.Serializers.Xml_FromStream_IndexViewModel
Before:

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Allocated
DataContractSerializer 85.41 μs 0.670 μs 0.594 μs 85.53 μs 83.64 μs 86.16 μs 4.4521 0.6849 37.89 KB
XmlSerializer 48.82 μs 0.372 μs 0.348 μs 48.71 μs 48.45 μs 49.51 μs 5.2376 0.7759 44.24 KB

After both PRs are in a branch:

Method Mean Error StdDev Median Min Max Gen 0 Gen 1 Allocated
DataContractSerializer 85.60 μs 0.227 μs 0.190 μs 85.56 μs 85.39 μs 86.07 μs 4.4353 0.6824 37.89 KB
XmlSerializer 47.76 μs 0.900 μs 0.925 μs 47.80 μs 45.41 μs 49.34 μs 5.2765 0.7817 43.63 KB

…ead of converting to string when value is primitive.
@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-Serialization labels Sep 11, 2022
@TrayanZapryanov TrayanZapryanov marked this pull request as draft September 11, 2022 13:22
_w.WriteValue((ushort)(object)value);
else if (typeof(T) == typeof(uint))
_w.WriteValue((uint)(object)value);
//else if (typeof(T) == typeof(ulong))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to support these also - we need to extend XmlWriter.WriteValue methods with ulong and guid

}

[RequiresUnreferencedCode("Calls WriteCheckDefault")]
private void WritePrimitive(string method, string name, string? ns, object? defaultValue, SourceInfo source, TypeMapping mapping, bool writeXsiType, bool isElement, bool isNullable)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

writeXsiType was always false in all call sites.
isElement was not used also

@TrayanZapryanov
Copy link
Contributor Author

Looks quote promising as results once both PRs are applied:
Benchmark :MicroBenchmarks.Serializers.Xml_ToStream_MyEventsListerViewModel
Before:

Method Mean Error StdDev Median Min Max Gen 0 Allocated
DataContractSerializer 401.8 μs 5.09 μs 4.25 μs 403.0 μs 389.9 μs 405.9 μs 11.3086 102.98 KB
XmlSerializer 251.3 μs 3.05 μs 2.55 μs 251.4 μs 243.6 μs 253.9 μs 8.0645 69.06 KB

After:

Method Mean Error StdDev Median Min Max Gen 0 Allocated
DataContractSerializer 415.2 μs 4.52 μs 4.23 μs 415.8 μs 401.3 μs 419.2 μs 11.6667 102.98 KB
XmlSerializer 250.9 μs 3.04 μs 2.54 μs 251.9 μs 245.0 μs 252.8 μs 4.0323 34.28 KB

@ghost ghost closed this Oct 12, 2022
@ghost
Copy link

ghost commented Oct 12, 2022

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 11, 2022
@TrayanZapryanov TrayanZapryanov deleted the fix_writing_primitive_types_in_serializers branch April 1, 2023 16:11
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Serialization community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants