-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Remove unsafe from DateTimeRawInfo #121479
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
Conversation
|
@EgorBot -amd -intel -arm using System;
using System.Collections.Generic;
using System.Globalization;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;
BenchmarkSwitcher.FromAssembly(typeof(DateTimeParsingBenchmarks).Assembly).Run(args);
public class DateTimeParsingBenchmarks
{
public static IEnumerable<object[]> TestData()
{
yield return ["2025-11-10", "yyyy-MM-dd", CultureInfo.InvariantCulture, DateTimeStyles.None];
yield return ["10/11/2025 14:37:05", "dd/MM/yyyy HH:mm:ss", new CultureInfo("en-GB"), DateTimeStyles.None];
yield return ["11/10/2025 2:37:05 PM", "M/d/yyyy h:mm:ss tt", new CultureInfo("en-US"), DateTimeStyles.AssumeLocal];
yield return ["Mon, 10 Nov 2025 14:37:05 GMT", "ddd, dd MMM yyyy HH':'mm':'ss 'GMT'", CultureInfo.InvariantCulture, DateTimeStyles.AdjustToUniversal | DateTimeStyles.AssumeUniversal];
yield return ["2025-11-10T14:37:05+01:00", "yyyy'-'MM'-'dd'T'HH':'mm':'ssK", CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind];
}
[Benchmark]
[ArgumentsSource(nameof(TestData))]
public DateTime TryParseExact_WithCases(string text, string format, CultureInfo culture, DateTimeStyles styles)
=> DateTime.TryParseExact(text, format, culture, styles, out var dt) ? dt : default;
} |
src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes the DateTimeParse class by converting data structures to more efficient representations and removing unsafe code. The changes improve memory layout and performance while maintaining the same functionality.
Key changes:
- Converts the state machine lookup table from a jagged 2D array (
DS[][]) to a flat 1DReadOnlySpan<DS>with a helper method for indexed access - Adds explicit base types to internal enums (
TM,DS,DTSubStringType) for better memory efficiency - Removes unsafe code by replacing
int*withInlineArray3<int>inDateTimeRawInfo
|
PTAL @stephentoub @tannergooding I removed an unsafe block and had to perform a few micro-optimizations to mitigate perf impact from bounds checking.
|
src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs
Outdated
Show resolved
Hide resolved
stephentoub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if there are no regressions
src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeParse.cs
Outdated
Show resolved
Hide resolved
|
/ba-g certificate issues |
Remove a bit of unsafe code from DateTimeRawInfo + some small perf tweaks to mitigate performance impact from bounds checks.