Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add JsonEnumMemberNameAttribute. #105032

Merged
merged 14 commits into from
Jul 22, 2024

Conversation

eiriktsarpalis
Copy link
Member

@eiriktsarpalis eiriktsarpalis commented Jul 17, 2024

Makes the following changes:

  1. Implements Support customizing enum member names in System.Text.Json #74385.
  2. Moves to using allocation-free enum parsing via the new alternate lookup types.
  3. Adds handling and tests for char-backed enum types that are possible in F#.

Fix #74385. Pending API review.

Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

1 similar comment
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@eiriktsarpalis eiriktsarpalis added this to the 9.0.0 milestone Jul 17, 2024
@eiriktsarpalis eiriktsarpalis self-assigned this Jul 17, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@jozkee jozkee 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.

eiriktsarpalis and others added 2 commits July 19, 2024 07:37
…ion/Converters/Value/EnumConverter.cs

Co-authored-by: David Cantú <dacantu@microsoft.com>
@eiriktsarpalis
Copy link
Member Author

I've reworked the parsing implementation and refined the casing semantics and identifier validation logics. The implementation also improves performance substantially as this benchmark shows:

Method Branch _value Mean Error StdDev Median Min Max Ratio MannWhitney(3%) Gen0 Allocated Alloc Ratio
Serialize main Default 40.85 ns 0.469 ns 0.439 ns 40.85 ns 40.29 ns 41.58 ns 1.00 Base 0.0007 - NA
Serialize PR Default 29.02 ns 0.097 ns 0.081 ns 28.99 ns 28.91 ns 29.16 ns 0.71 Faster - - NA
Deserialize main Default 82.40 ns 0.232 ns 0.194 ns 82.32 ns 82.17 ns 82.77 ns 1.00 Base - - NA
Deserialize PR Default 81.06 ns 0.146 ns 0.137 ns 81.03 ns 80.86 ns 81.29 ns 0.98 Same - - NA
SerializeDictionary main Default 195.27 ns 1.157 ns 1.082 ns 195.12 ns 193.91 ns 197.35 ns 1.00 Base 0.0036 112 B 1.00
SerializeDictionary PR Default 153.45 ns 1.738 ns 1.626 ns 152.94 ns 151.90 ns 156.54 ns 0.79 Faster 0.0021 - 0.00
Serialize main Insta(...)ublic [27] 39.51 ns 0.388 ns 0.344 ns 39.45 ns 39.09 ns 40.18 ns 1.00 Base 0.0008 24 B 1.00
Serialize PR Insta(...)ublic [27] 34.98 ns 0.171 ns 0.151 ns 34.93 ns 34.84 ns 35.38 ns 0.89 Faster - - 0.00
Deserialize main Insta(...)ublic [27] 130.20 ns 0.413 ns 0.366 ns 130.10 ns 129.73 ns 130.92 ns 1.00 Base - - NA
Deserialize PR Insta(...)ublic [27] 94.75 ns 1.139 ns 1.010 ns 94.93 ns 93.47 ns 96.49 ns 0.73 Faster - - NA
SerializeDictionary main Insta(...)ublic [27] 456.33 ns 5.461 ns 5.108 ns 457.01 ns 448.09 ns 466.25 ns 1.00 Base 0.0196 528 B 1.00
SerializeDictionary PR Insta(...)ublic [27] 333.09 ns 1.271 ns 1.127 ns 332.90 ns 331.50 ns 335.34 ns 0.73 Faster 0.0062 - 0.00
Serialize main 2147483647 105.37 ns 1.391 ns 1.233 ns 105.35 ns 102.84 ns 107.50 ns 1.00 Base 0.0052 - NA
Serialize PR 2147483647 52.69 ns 0.148 ns 0.124 ns 52.68 ns 52.51 ns 52.96 ns 0.50 Faster - - NA
Deserialize main 2147483647 71.29 ns 0.125 ns 0.098 ns 71.27 ns 71.15 ns 71.50 ns 1.00 Base - - NA
Deserialize PR 2147483647 71.19 ns 0.156 ns 0.130 ns 71.19 ns 70.94 ns 71.46 ns 1.00 Same - - NA
SerializeDictionary main 2147483647 234.79 ns 2.926 ns 2.594 ns 234.70 ns 230.67 ns 239.88 ns 1.00 Base 0.0067 192 B 1.00
SerializeDictionary PR 2147483647 118.84 ns 0.302 ns 0.282 ns 118.79 ns 118.47 ns 119.37 ns 0.51 Faster - - 0.00

@eiriktsarpalis eiriktsarpalis added the tenet-performance Performance related issue label Jul 19, 2024
@eiriktsarpalis eiriktsarpalis merged commit 78db74f into dotnet:main Jul 22, 2024
80 of 84 checks passed
@eiriktsarpalis eiriktsarpalis deleted the jsonenummembernameattribute branch July 22, 2024 19:09
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support customizing enum member names in System.Text.Json
5 participants