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

Implement IUtf8SpanFormattable on DateTime, DateTimeOffset, DateOnly, TimeOnly, TimeSpan, Char, Rune #84469

Merged
merged 3 commits into from
Apr 9, 2023

Conversation

stephentoub
Copy link
Member

Contributes to #81500

(Note that we have a fair amount of code duplication between various formatters (like the DateTimeFormat helper updated in this PR), FormattingHelpers used by Utf8Formatter, and Number.Formatting.cs used by formatting for our primitives. I've done a little consolidation in this PR, but there's a lot more than can / should be done. We're also inconsistent as to whether we're working in terms of spans, refs, or pointers, which leads to more duplication. That can also be consolidated further subsequently.)

@dotnet-issue-labeler
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.

@ghost
Copy link

ghost commented Apr 7, 2023

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

Issue Details

Contributes to #81500

(Note that we have a fair amount of code duplication between various formatters (like the DateTimeFormat helper updated in this PR), FormattingHelpers used by Utf8Formatter, and Number.Formatting.cs used by formatting for our primitives. I've done a little consolidation in this PR, but there's a lot more than can / should be done. We're also inconsistent as to whether we're working in terms of spans, refs, or pointers, which leads to more duplication. That can also be consolidated further subsequently.)

Author: stephentoub
Assignees: -
Labels:

area-System.Runtime

Milestone: 8.0.0

Also dedup Utf8Formatter for TimeSpan with TimeSpan's new IUtf8SpanFormattable implementation and a little more cleanup.

And fix parameter name of TryFormat to match approved name.
@stephentoub
Copy link
Member Author

private DateTime _dt = new DateTime(2023, 04, 06, 10, 37, 12, DateTimeKind.Utc);
private TimeSpan _ts = TimeSpan.FromSeconds(12345.6789);
private char[] _chars = new char[100];
private byte[] _bytes = new byte[100];

[Benchmark] public bool DT_Char_R() => _dt.TryFormat(_chars, out _, "r");
[Benchmark] public bool DT_Char_O() => _dt.TryFormat(_chars, out _, "o");
[Benchmark] public bool DT_Char_Rfc1123() => _dt.TryFormat(_chars, out _, DateTimeFormatInfo.InvariantInfo.RFC1123Pattern);
[Benchmark] public bool TS_Char_C() => _ts.TryFormat(_chars, out _, "c");
[Benchmark] public bool TS_Char_G() => _ts.TryFormat(_chars, out _, "G");

[Benchmark] public bool UTF8Formatter_DT_R() => Utf8Formatter.TryFormat(_dt, _bytes, out _, new StandardFormat('R'));
[Benchmark] public bool UTF8Formatter_DT_O() => Utf8Formatter.TryFormat(_dt, _bytes, out _, new StandardFormat('O'));
[Benchmark] public bool UTF8Formatter_TS_C() => Utf8Formatter.TryFormat(_ts, _bytes, out _, default);
[Benchmark] public bool UTF8Formatter_TS_G() => Utf8Formatter.TryFormat(_ts, _bytes, out _, new StandardFormat('G'));
Method Toolchain Mean Error StdDev Ratio
DT_Char_R \main\corerun.exe 21.79 ns 0.220 ns 0.206 ns 1.00
DT_Char_R \pr\corerun.exe 19.25 ns 0.376 ns 0.352 ns 0.88
DT_Char_O \main\corerun.exe 27.67 ns 0.171 ns 0.151 ns 1.00
DT_Char_O \pr\corerun.exe 24.99 ns 0.162 ns 0.144 ns 0.90
DT_Char_Rfc1123 \main\corerun.exe 179.71 ns 1.347 ns 1.194 ns 1.00
DT_Char_Rfc1123 \pr\corerun.exe 175.81 ns 1.055 ns 0.881 ns 0.98
TS_Char_C \main\corerun.exe 23.47 ns 0.176 ns 0.164 ns 1.00
TS_Char_C \pr\corerun.exe 21.51 ns 0.127 ns 0.112 ns 0.92
TS_Char_G \main\corerun.exe 33.72 ns 0.210 ns 0.186 ns 1.00
TS_Char_G \pr\corerun.exe 32.16 ns 0.666 ns 0.712 ns 0.96
UTF8Formatter_DT_R \main\corerun.exe 18.47 ns 0.101 ns 0.095 ns 1.00
UTF8Formatter_DT_R \pr\corerun.exe 18.60 ns 0.075 ns 0.062 ns 1.01
UTF8Formatter_DT_O \main\corerun.exe 23.76 ns 0.095 ns 0.084 ns 1.00
UTF8Formatter_DT_O \pr\corerun.exe 25.30 ns 0.084 ns 0.078 ns 1.06
UTF8Formatter_TS_C \main\corerun.exe 21.20 ns 0.130 ns 0.115 ns 1.00
UTF8Formatter_TS_C \pr\corerun.exe 19.19 ns 0.062 ns 0.058 ns 0.90
UTF8Formatter_TS_G \main\corerun.exe 22.57 ns 0.254 ns 0.212 ns 1.00
UTF8Formatter_TS_G \pr\corerun.exe 22.44 ns 0.071 ns 0.060 ns 0.99

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.

5 participants