Add missing fast path for DateTime format "G"#129374
Conversation
Mirrors the existing single-char fast paths for 'o', 'r', 's', 'u' and the null/empty-format invariant fast path that ToString(InvariantCulture) already uses, routing through TryFormatInvariantG instead of FormatCustomized. Output is byte-identical to the existing pattern-based path ("MM/dd/yyyy HH:mm:ss", 19 chars). ~5.5x faster (41.58 ns -> 7.54 ns median).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/area-system-datetime |
There was a problem hiding this comment.
Pull request overview
Adds an invariant-culture fast path for the standard "G" format when formatting a DateTime (i.e., offset.Ticks == NullOffset) by directly using the existing TryFormatInvariantG implementation. This aligns "G" with the existing single-character fast paths (e.g., "o", "r", "s", "u") and the already-existing null/empty-format invariant fast path.
Changes:
- Add a
"G"switch case inDateTimeFormat.Formatthat fast-pathsDateTime.ToString("G", InvariantCulture)toTryFormatInvariantG. - Add the corresponding
"G"fast path inDateTimeFormat.TryFormat<TChar>for invariant culture +DateTime(NullOffset) scenarios.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeFormat.cs | Adds an invariant "G" fast path in both Format and TryFormat<TChar> using TryFormatInvariantG when offset is NullOffset. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
| if (offset.Ticks == NullOffset && ReferenceEquals(dtfi, DateTimeFormatInfo.InvariantInfo)) | ||
| { | ||
| str = string.FastAllocateString(FormatInvariantGMinLength); | ||
| TryFormatInvariantG(dateTime, offset, new Span<char>(ref str.GetRawStringData(), str.Length), out charsWritten); |
There was a problem hiding this comment.
fyi: this PR introduces a new unsafe code, GetRawStringData will start requiring unsafe{} scope soon. We try to remove it from BCL everywhere, adding a new one is a step in the opposite direction
There was a problem hiding this comment.
ah! great point -- I was just copying existing pattern. I can update all these -- do you have a suggested new pattern, or guidance? I seem to remember a document, I'll find it.
There was a problem hiding this comment.
ah, I found the docs, but any guidance welcome.
There was a problem hiding this comment.
ah I think just string.Create. Remeasuring.
There was a problem hiding this comment.
TBH, I am not fully sure, @jkotas I think we had a type-loading/rooting issue with String.Create recently? But generally, yes, string.Create is the way to go, it's just that it'd be nice if it could inline the lambda.
Replace string.FastAllocateString + GetRawStringData with string.Create<TState> in all single-character fast paths in DateTimeFormat.Format (null-format Invariant and IsTimeOnlySpecialCase, DateTimeOffset null+Invariant, 'r'/'R', 's', 'u', and the new 'G'). The previous pattern exposes raw string mutation; string.Create is the idiomatic safe equivalent recommended by the .NET memory-safety design (dotnet/designs#2025/memory-safety) for new code. Also drop the unsafe modifier on Format() since its body no longer uses any unsafe constructs.
Adds a fast path for
DateTime.ToString("G", CultureInfo.InvariantCulture)similar to existing single-char fast path that'o','r','s', and'u'already have inDateTimeFormat.Format/TryFormat<TChar>, and identical to the one the null/empty-format case (dt.ToString(CultureInfo.InvariantCulture)) already takes viaTryFormatInvariantG.The fast path is gated by
offset.Ticks == NullOffset && ReferenceEquals(dtfi, DateTimeFormatInfo.InvariantInfo), soDateTimeOffset.ToString("G", InvariantCulture)and any custom/clonedDateTimeFormatInfofall through to the existing pattern-based path unchanged.Perf Results
5.5x faster
ResultsComparer, threshold 1%, noise 1 ns, 5 launches × 15 iterations × 16384 invocations, P-core pinned on i9-14900K:The remaining 19
Perf_DateTime.ToString*cases (other formats, current-culture,DateTimeOffset) are unchanged. Allocation unchanged everywhere.ToStringInvariant(format)is a new benchmark inPerf_DateTime.csthat callsdate.ToString(format, CultureInfo.InvariantCulture).Tests
Output is byte-identical to the slow path for
DateTime+ invariant culture ("MM/dd/yyyy HH:mm:ss", 19 chars). ExistingDateTime/DateTimeOffsetformatting tests pass. In addition, an ad-hoc correctness harness compared baseline vs. modified outputs over 10 DateTimes (incl.MinValue,MaxValue, year 1, year 9999, Feb 29 2000, allDateTimeKinds) × 5 cultures × 16 formats × {ToString,TryFormat} plus DateTimeOffset equivalents — 1040 byte-identical outputs. Separately verifiedTryFormatwith too-small destinations (sizes 0..20 char, 0..25 UTF-8), composite formatting$"{dt:G}", cloned DTFI, and DTFI with customizedLongTimePattern— all behave identically to baseline.Note
This PR description was drafted by GitHub Copilot.