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

Avoid ToCharArray allocation in JsonCamelCaseNamingPolicy #41363

Merged
stephentoub merged 1 commit into from
Sep 26, 2019
Merged

Avoid ToCharArray allocation in JsonCamelCaseNamingPolicy #41363

stephentoub merged 1 commit into from
Sep 26, 2019

Conversation

@stephentoub
Copy link
Member

@stephentoub stephentoub commented Sep 26, 2019

Microbenchmark calling ConvertName with either "PascalCase" or "camelCased":

| Method |       Text |      Mean |     Error |    StdDev |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|------- |----------- |----------:|----------:|----------:|-------:|------:|------:|----------:|
|    Old | PascalCase | 35.724 ns | 1.3263 ns | 3.7410 ns | 0.0458 |     - |     - |      96 B |
|    New | PascalCase | 26.932 ns | 0.7734 ns | 2.2683 ns | 0.0229 |     - |     - |      48 B |
|    Old | camelCased |  3.323 ns | 0.1484 ns | 0.4211 ns |      - |     - |     - |         - |
|    New | camelCased |  2.685 ns | 0.1435 ns | 0.4186 ns |      - |     - |     - |         - |

cc: @steveharter, @ahsonkhan

@stephentoub stephentoub added this to the 5.0 milestone Sep 26, 2019
ahsonkhan
ahsonkhan reviewed Sep 26, 2019
View reviewed changes
src/System.Text.Json/src/System/Text/Json/Serialization/JsonCamelCaseNamingPolicy.cs
FixCasing(chars);
});
#else
char[] chars = name.ToCharArray();
Copy link

@ahsonkhan ahsonkhan Sep 26, 2019

Choose a reason for hiding this comment

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

Given property/key names are almost always small, is it worth it to copy the contents into a stackalloc span instead of always calling ToCharArray for say < 256?

Copy link
Member Author

@stephentoub stephentoub Sep 26, 2019

Choose a reason for hiding this comment

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

Maybe? Feel free to look at that separately; I'm going to skip it for this PR. I'm also not sure how much time we want to invest in the OOBs.

@ahsonkhan
Copy link

ahsonkhan commented Sep 26, 2019 •
edited
Loading

Candidate to port to 3.1 as part of the perf push.

@stephentoub stephentoub merged commit 4cfc7ec into dotnet:master Sep 26, 2019
@stephentoub stephentoub deleted the casingalloc branch September 26, 2019 21:30
steveharter added a commit to steveharter/dotnet_corefx that referenced this pull request Oct 14, 2019
@steveharter
Avoid ToCharArray allocation in JsonCamelCaseNamingPolicy (dotnet#41363)
ec989cb
steveharter added a commit to steveharter/dotnet_corefx that referenced this pull request Oct 14, 2019
@steveharter
Avoid ToCharArray allocation in JsonCamelCaseNamingPolicy (dotnet#41363)
002ab8a
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@stephentoub
Avoid ToCharArray allocation in JsonCamelCaseNamingPolicy (dotnet/cor…
bdb1bb6 
…efx#41363)

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

Reviewers

1 more reviewer

@ahsonkhan ahsonkhan ahsonkhan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Text.Json tenet-performance Performance related issue

Projects

None yet

Milestone

5.0

Development

Successfully merging this pull request may close these issues.

2 participants

@stephentoub @ahsonkhan