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

Use JSON source generator #87

Merged
merged 1 commit into from
Oct 22, 2022

Conversation

jnyrup
Copy link
Contributor

@jnyrup jnyrup commented Oct 3, 2022

With the switch to System.Text.Json in #85 we can further improve TimeZoneNames for NativeAOT applications by switching to a source generated TimeZoneData JSON deserializer.

Unfortunately System.Text.Json does not (yet) support init only properties, see dotnet/runtime#58770.
For this draft I went with public setters, but alternatively a parameterized constructor should also work.

When I ran DataBuilder the order of the output changed, which I guess is either due non-stable dictionary sorting or that the CurrentCulture on my machine is a mix of en-US/en-GB and da-DK (reported as en-DK).

I made a quick benchmark on calling TimeZoneData.Load() on .NET6 using Benchmarkdotnet which only focuses on the performance benefits of switching to a source generated JSON deserializer.

branch Mean Error StdDev Ratio
main 174.5 ms 2.98 ms 2.49 ms 1.00
PR 114.8 ms 2.25 ms 2.10 ms 0.66

Some stats on a TimeZoneNames.NativeAot application.

main:

  • File size of TimeZoneNames.NativeAot.exe is 6091 KB
  • Crashes at runtime with
Unhandled Exception: System.TypeInitializationException: TypeInitialization_Type_NoTypeAvailable
	---> System.NotSupportedException: DeserializeNoConstructor, JsonConstructorAttribute, TimeZoneNames.TimeZoneData Path: $ | LineNumber: 0 | BytePositionInLine: 1.
	---> System.NotSupportedException: DeserializeNoConstructor, JsonConstructorAttribute, TimeZoneNames.TimeZoneData

PR:

  • File size of TimeZoneNames.NativeAot.exe is 4420 KB
  • Works at runtime.

@jnyrup jnyrup marked this pull request as ready for review October 19, 2022 16:02
@mattjohnsonpint mattjohnsonpint merged commit 0ca6855 into mattjohnsonpint:main Oct 22, 2022
@mattjohnsonpint
Copy link
Owner

Thanks!

@jnyrup jnyrup deleted the trimmable_lib2 branch October 22, 2022 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants