AArnott / Nerdbank.MessagePack

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

Leverage C# 13 to simplify and optimize async serialization #81

Closed AArnott closed 2 weeks ago

AArnott commented 2 weeks ago

Summary

DeserializeAsync obviously needs more work, but there are plenty opportunities for improvement.

In the following metrics, pay attention to the improved ratios instead of looking at the Mean column, which shows significant regressions for unchanged code paths and it didn't repro later.

Before

Method Categories Mean Error StdDev Ratio RatioSD
DeserializeAsArrayInit array-init,Deserialize 54.43 ns 0.289 ns 0.256 ns 1.00 0.01
DeserializeAsyncAsArrayInit array-init,Deserialize 637.12 ns 5.798 ns 5.423 ns 11.70 0.11
SerializeAsArrayInit array-init,Serialize 86.64 ns 1.101 ns 1.030 ns 1.00 0.02
SerializeAsyncAsArrayInit array-init,Serialize 107.98 ns 0.843 ns 0.704 ns 1.25 0.02
DeserializeAsArray array,Deserialize 52.79 ns 0.608 ns 0.569 ns 1.00 0.01
DeserializeAsyncAsArray array,Deserialize 622.12 ns 7.645 ns 7.151 ns 11.79 0.18
SerializeAsArray array,Serialize 80.59 ns 1.099 ns 1.028 ns 1.00 0.02
SerializeAsyncAsArray array,Serialize 100.56 ns 1.927 ns 1.802 ns 1.25 0.03
DeserializeMapInit map-init,Deserialize 103.90 ns 1.047 ns 0.979 ns 1.00 0.01
DeserializeAsyncMapInit map-init,Deserialize 1,168.60 ns 23.107 ns 54.466 ns 11.25 0.53
SerializeMapInit map-init,Serialize 97.32 ns 1.033 ns 0.967 ns 1.00 0.01
SerializeAsyncMapInit map-init,Serialize 161.89 ns 2.195 ns 2.054 ns 1.66 0.03
DeserializeMap map,Deserialize 100.33 ns 0.610 ns 0.541 ns 1.00 0.01
DeserializeAsyncMap map,Deserialize 1,040.28 ns 20.102 ns 19.743 ns 10.37 0.20
SerializeMap map,Serialize 99.47 ns 0.667 ns 0.624 ns 1.00 0.01
SerializeAsyncMap map,Serialize 159.49 ns 1.741 ns 1.628 ns 1.60 0.02

After

Method Categories Mean Error StdDev Ratio RatioSD
DeserializeAsArrayInit array-init,Deserialize 94.40 ns 1.886 ns 4.372 ns 1.00 0.06
DeserializeAsyncAsArrayInit array-init,Deserialize 758.08 ns 14.929 ns 24.529 ns 8.05 0.45
SerializeAsArrayInit array-init,Serialize 145.46 ns 2.902 ns 6.058 ns 1.00 0.06
SerializeAsyncAsArrayInit array-init,Serialize 136.35 ns 2.720 ns 3.023 ns 0.94 0.04
DeserializeAsArray array,Deserialize 92.38 ns 1.820 ns 4.071 ns 1.00 0.06
DeserializeAsyncAsArray array,Deserialize 749.10 ns 14.508 ns 19.859 ns 8.12 0.41
SerializeAsArray array,Serialize 136.23 ns 2.293 ns 2.144 ns 1.00 0.02
SerializeAsyncAsArray array,Serialize 140.85 ns 2.784 ns 5.750 ns 1.03 0.04
DeserializeMapInit map-init,Deserialize 188.56 ns 3.727 ns 9.281 ns 1.00 0.07
DeserializeAsyncMapInit map-init,Deserialize 801.47 ns 15.379 ns 15.794 ns 4.26 0.23
SerializeMapInit map-init,Serialize 170.40 ns 3.343 ns 4.463 ns 1.00 0.04
SerializeAsyncMapInit map-init,Serialize 162.65 ns 3.241 ns 5.761 ns 0.96 0.04
DeserializeMap map,Deserialize 181.09 ns 3.359 ns 7.444 ns 1.00 0.06
DeserializeAsyncMap map,Deserialize 800.18 ns 15.741 ns 30.327 ns 4.43 0.24
SerializeMap map,Serialize 165.76 ns 2.583 ns 2.416 ns 1.00 0.02
SerializeAsyncMap map,Serialize 166.74 ns 2.860 ns 2.676 ns 1.01 0.02
codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 56.59341% with 79 lines in your changes missing coverage. Please review.

Project coverage is 75.00%. Comparing base (e3a73d5) to head (6c2fdb1). Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ank.MessagePack/Converters/ObjectMapConverter`1.cs 52.77% 13 Missing and 4 partials :warning:
...k.MessagePack/Converters/ObjectArrayConverter`1.cs 70.00% 6 Missing and 6 partials :warning:
...nverters/ObjectMapWithNonDefaultCtorConverter`2.cs 47.82% 10 Missing and 2 partials :warning:
src/Nerdbank.MessagePack/MessagePackAsyncReader.cs 57.69% 10 Missing and 1 partial :warning:
...erdbank.MessagePack/Converters/ArrayConverter`1.cs 36.36% 7 Missing :warning:
...erters/ObjectArrayWithNonDefaultCtorConverter`2.cs 66.66% 4 Missing and 3 partials :warning:
...nk.MessagePack/Converters/DictionaryConverter`3.cs 0.00% 6 Missing :warning:
...nk.MessagePack/Converters/EnumerableConverter`2.cs 0.00% 5 Missing :warning:
src/Nerdbank.MessagePack/MessagePackAsyncWriter.cs 0.00% 0 Missing and 1 partial :warning:
src/Nerdbank.MessagePack/StandardVisitor.cs 80.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #81 +/- ## ========================================== - Coverage 76.37% 75.00% -1.37% ========================================== Files 115 115 Lines 6746 6837 +91 Branches 924 960 +36 ========================================== - Hits 5152 5128 -24 - Misses 1304 1415 +111 - Partials 290 294 +4 ``` | [Flag](https://app.codecov.io/gh/AArnott/Nerdbank.MessagePack/pull/81/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Arnott) | Coverage Δ | | |---|---|---| | [Linux](https://app.codecov.io/gh/AArnott/Nerdbank.MessagePack/pull/81/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Arnott) | `75.00% <56.59%> (-1.37%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrew+Arnott#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AArnott commented 2 weeks ago

Code coverage for the async paths isn't where it should be by the time we stabilize. But I'm holding out writing tests to specifically cover these lines until I'm sure those lines will stick around.