Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Add (de)serialization support for Base64 string as Byte array#38106

Merged
ahsonkhan merged 3 commits intodotnet:masterfrom
steveharter:Base64Serializer
May 31, 2019
Merged

Add (de)serialization support for Base64 string as Byte array#38106
ahsonkhan merged 3 commits intodotnet:masterfrom
steveharter:Base64Serializer

Conversation

@steveharter
Copy link
Copy Markdown
Contributor

@steveharter steveharter commented May 31, 2019 •
edited by ahsonkhan
Loading

Uptake the recent Base64 reader\writer work to the serializer.

Addresses serialization work for #36974 and follow up for reader\writer\doc PR.

@steveharter steveharter self-assigned this May 31, 2019
@ahsonkhan ahsonkhan added this to the 3.0 milestone May 31, 2019
ahsonkhan
ahsonkhan approved these changes May 31, 2019
View reviewed changes
Copy link
Copy Markdown

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM.

Comment thread src/System.Text.Json/src/System.Text.Json.csproj Outdated
<Compile Include="System\Text\Json\JsonHelpers.cs" />
<Compile Include="System\Text\Json\JsonHelpers.Date.cs" />
<Compile Include="System\Text\Json\JsonTokenType.cs" />
<Compile Include="System\Text\Json\Serialization\Converters\JsonValueConverterByteArray.cs" />
Copy link
Copy Markdown

@ahsonkhan ahsonkhan May 31, 2019

Choose a reason for hiding this comment

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

nit: sort order

Comment thread src/System.Text.Json/tests/Serialization/Array.ReadTests.cs
}

[Fact]
public static void ReadByteArray()
Copy link
Copy Markdown

@ahsonkhan ahsonkhan May 31, 2019

Choose a reason for hiding this comment

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

Can you add a test for reading invalid base 64, like "A==="?

@steveharter
Copy link
Copy Markdown
Contributor Author

steveharter commented May 31, 2019

Some perf numbers for an object containing a single 1K byte[] property. It basically avoids 1,023 extra loops through the serializer\reader\writer. In this case, 40x faster on serialize and 11x faster on deserialize.

Before:

|               Method |     Mean |    Error |   StdDev |   Median |      Min |      Max | Gen 0/1k Op | Gen 1/1k Op | Gen 2/1k Op | Allocated Memory/Op |
|--------------------- |---------:|---------:|---------:|---------:|---------:|---------:|------------:|------------:|------------:|--------------------:|
|    SerializeToString | 174.0 us | 3.322 us | 3.107 us | 173.0 us | 169.9 us | 179.7 us |      7.5549 |           - |           - |            31.36 KB |
| SerializeToUtf8Bytes | 173.3 us | 5.316 us | 4.439 us | 172.7 us | 168.4 us | 185.8 us |      6.7935 |           - |           - |            27.78 KB |
|    SerializeToStream | 172.0 us | 2.323 us | 2.173 us | 171.0 us | 168.6 us | 176.7 us |      5.3763 |           - |           - |            24.17 KB |

|                   Method |     Mean |    Error |   StdDev |   Median |      Min |      Max | Gen 0/1k Op | Gen 1/1k Op | Gen 2/1k Op | Allocated Memory/Op |
|------------------------- |---------:|---------:|---------:|---------:|---------:|---------:|------------:|------------:|------------:|--------------------:|
|    DeserializeFromString | 93.11 us | 1.686 us | 1.577 us | 92.42 us | 90.26 us | 95.99 us |      1.4556 |           - |           - |             6.92 KB |
| DeserializeFromUtf8Bytes | 91.42 us | 1.731 us | 1.852 us | 91.51 us | 84.85 us | 93.43 us |      0.7366 |           - |           - |             3.31 KB |
|    DeserializeFromStream | 93.66 us | 1.856 us | 1.645 us | 93.68 us | 91.16 us | 96.39 us |      0.7353 |           - |           - |             3.38 KB |

After:

|               Method |     Mean |     Error |    StdDev |   Median |      Min |      Max | Gen 0/1k Op | Gen 1/1k Op | Gen 2/1k Op | Allocated Memory/Op |
|--------------------- |---------:|----------:|----------:|---------:|---------:|---------:|------------:|------------:|------------:|--------------------:|
|    SerializeToString | 4.278 us | 0.0499 us | 0.0466 us | 4.282 us | 4.186 us | 4.337 us |      0.7946 |           - |           - |              3360 B |
| SerializeToUtf8Bytes | 4.035 us | 0.0793 us | 0.0814 us | 4.057 us | 3.902 us | 4.186 us |      0.4213 |           - |           - |              1768 B |
|    SerializeToStream | 4.429 us | 0.4654 us | 0.5360 us | 4.376 us | 3.783 us | 5.627 us |      0.0310 |           - |           - |               144 B |

|                   Method |     Mean |     Error |    StdDev |   Median |      Min |       Max | Gen 0/1k Op | Gen 1/1k Op | Gen 2/1k Op | Allocated Memory/Op |
|------------------------- |---------:|----------:|----------:|---------:|---------:|----------:|------------:|------------:|------------:|--------------------:|
|    DeserializeFromString | 8.729 us | 0.1878 us | 0.1928 us | 8.704 us | 8.312 us |  9.160 us |      0.6187 |           - |           - |             2.63 KB |
| DeserializeFromUtf8Bytes | 8.530 us | 0.7706 us | 0.8875 us | 7.995 us | 7.815 us | 10.243 us |      0.2545 |           - |           - |             1.05 KB |
|    DeserializeFromStream | 8.210 us | 0.1232 us | 0.1092 us | 8.175 us | 8.054 us |  8.446 us |      0.2619 |           - |           - |             1.12 KB |

Comparison:

| Faster                                                                           | base/diff | Base Median (ns) | Diff Median (ns) | Modality  |
| -------------------------------------------------------------------------------- | ---------:| ----------------:| ----------------:| ---------- |
| System.Text.Json.Serialization.Tests.WriteJson<ByteArrayModel>.SerializeToUtf8By |     42.56 |        172658.78 |          4057.12 |           |
| System.Text.Json.Serialization.Tests.WriteJson<ByteArrayModel>.SerializeToString |     40.39 |        172964.07 |          4282.40 |           |
| System.Text.Json.Serialization.Tests.WriteJson<ByteArrayModel>.SerializeToStream |     39.07 |        170961.97 |          4375.60 | multimodal|
| System.Text.Json.Serialization.Tests.ReadJson<ByteArrayModel>.DeserializeFromStr |     11.46 |         93683.35 |          8175.43 |           |
| System.Text.Json.Serialization.Tests.ReadJson<ByteArrayModel>.DeserializeFromUtf |     11.45 |         91506.72 |          7995.23 |           |
| System.Text.Json.Serialization.Tests.ReadJson<ByteArrayModel>.DeserializeFromStr |     10.62 |         92420.52 |          8704.45 |           |

@ahsonkhan
Copy link
Copy Markdown

ahsonkhan commented May 31, 2019 •
edited
Loading

The CI was "green" prior to the nits above, so this is safe to merge now.

@ahsonkhan ahsonkhan merged commit a7ad25c into dotnet:master May 31, 2019
@steveharter steveharter deleted the Base64Serializer branch May 31, 2019 19:05
@eiriktsarpalis eiriktsarpalis mentioned this pull request Mar 17, 2021
Closed
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@steveharter @ahsonkhan
Add (de)serialization support for Base64 string as Byte array (dotnet…
bc3e331 
…/corefx#38106)

* Add (de)serialization for Base64 string as Byte array

* Review feedback


Commit migrated from dotnet/corefx@a7ad25c
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JeremyKuhne JeremyKuhne Awaiting requested review from JeremyKuhne

1 more reviewer

@ahsonkhan ahsonkhan ahsonkhan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@steveharter steveharter

Labels

area-System.Text.Json

Projects

None yet

Milestone

3.0

Development

Successfully merging this pull request may close these issues.

2 participants

@steveharter @ahsonkhan