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

Optimize DateTimeOffset #111112

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static DateTime UtcNow
private static ulong ToFileTimeLeapSecondsAware(long ticks) => default;

// IsValidTimeWithLeapSeconds is not expected to be called at all for now on non-Windows platforms
internal static bool IsValidTimeWithLeapSeconds(int year, int month, int day, int hour, int minute, DateTimeKind kind) => false;
internal static bool IsValidTimeWithLeapSeconds(DateTime value) => false;
pentp marked this conversation as resolved.
Show resolved Hide resolved

#pragma warning restore IDE0060
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,28 @@ public static unsafe DateTime UtcNow
}

[MethodImpl(MethodImplOptions.NoInlining)]
internal static unsafe bool IsValidTimeWithLeapSeconds(int year, int month, int day, int hour, int minute, DateTimeKind kind)
private static unsafe bool IsValidTimeWithLeapSeconds(DateTime value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely a deoptimization. Decomposing DateTime has its overhead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is never called in practice. And hopefully there will be no more leap seconds added in the future.

{
Interop.Kernel32.SYSTEMTIME time;
value.GetDate(out int year, out int month, out int day);
time.Year = (ushort)year;
time.Month = (ushort)month;
time.DayOfWeek = 0; // ignored by TzSpecificLocalTimeToSystemTime/SystemTimeToFileTime
time.Day = (ushort)day;
value.GetTime(out int hour, out int minute, out _);
time.Hour = (ushort)hour;
time.Minute = (ushort)minute;
time.Second = 60;
time.Milliseconds = 0;

if (kind != DateTimeKind.Utc)
if (value.Kind != DateTimeKind.Utc)
{
Interop.Kernel32.SYSTEMTIME st;
if (Interop.Kernel32.TzSpecificLocalTimeToSystemTime(IntPtr.Zero, &time, &st) != Interop.BOOL.FALSE)
return true;
}

if (kind != DateTimeKind.Local)
if (value.Kind != DateTimeKind.Local)
{
ulong ft;
if (Interop.Kernel32.SystemTimeToFileTime(&time, &ft) != Interop.BOOL.FALSE)
Expand All @@ -82,7 +84,7 @@ private static unsafe DateTime FromFileTimeLeapSecondsAware(ulong fileTime)

private static unsafe ulong ToFileTimeLeapSecondsAware(long ticks)
{
DateTime dt = new(ticks);
DateTime dt = new((ulong)ticks);
Interop.Kernel32.SYSTEMTIME time;

dt.GetDate(out int year, out int month, out int day);
Expand Down
209 changes: 97 additions & 112 deletions src/libraries/System.Private.CoreLib/src/System/DateTime.cs

Large diffs are not rendered by default.

370 changes: 162 additions & 208 deletions src/libraries/System.Private.CoreLib/src/System/DateTimeOffset.cs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/libraries/System.Private.CoreLib/src/System/Enum.cs
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ private static bool TryParseByValueOrName<TUnderlying, TStorage>(
return TryParseByName(enumType, value, ignoreCase, throwOnFailure, out Unsafe.As<TUnderlying, TStorage>(ref result));
}

NumberFormatInfo numberFormat = CultureInfo.InvariantCulture.NumberFormat;
NumberFormatInfo numberFormat = NumberFormatInfo.InvariantInfo;
const NumberStyles NumberStyle = NumberStyles.AllowLeadingSign | NumberStyles.AllowTrailingWhite;

Number.ParsingStatus status = Number.TryParseBinaryIntegerStyle(value, NumberStyle, numberFormat, out result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,23 @@ public static DateTimeFormatInfo CurrentInfo
}
}

public static DateTimeFormatInfo GetInstance(IFormatProvider? provider) =>
provider == null ? CurrentInfo :
provider is CultureInfo cultureProvider && !cultureProvider._isInherited ? cultureProvider.DateTimeFormat :
provider is DateTimeFormatInfo info ? info :
provider.GetFormat(typeof(DateTimeFormatInfo)) is DateTimeFormatInfo info2 ? info2 :
CurrentInfo; // Couldn't get anything, just use currentInfo as fallback
public static DateTimeFormatInfo GetInstance(IFormatProvider? provider)
{
return provider == null ? CurrentInfo : GetProviderNonNull(provider);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be optimizing for the case where provider is null at the expense of more code at the call site... why is that desirable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this approach from NumberFormatInfo which looks like it has been more thoroughly optimized. It adds a small code size increase on some callsites (but not all - in case of const provider = null it's an improvement).


static DateTimeFormatInfo GetProviderNonNull(IFormatProvider provider)
{
if (provider.GetType() == typeof(CultureInfo) && ((CultureInfo)provider)._dateTimeInfo is { } info)
{
return info;
}

return
provider as DateTimeFormatInfo ??
provider.GetFormat(typeof(DateTimeFormatInfo)) as DateTimeFormatInfo ??
CurrentInfo;
}
}

public object? GetFormat(Type? formatType)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ internal sealed class GregorianCalendarHelper
private readonly int m_minYear;
private readonly Calendar m_Cal;
private readonly EraInfo[] m_EraInfo;
private readonly long _minSupportedTicks;
private readonly long _maxSupportedTicks;

// Construct an instance of gregorian calendar.
internal GregorianCalendarHelper(Calendar cal, EraInfo[] eraInfo)
Expand All @@ -66,6 +68,8 @@ internal GregorianCalendarHelper(Calendar cal, EraInfo[] eraInfo)
m_EraInfo = eraInfo;
m_maxYear = eraInfo[0].maxEraYear;
m_minYear = eraInfo[0].minEraYear;
_minSupportedTicks = cal.MinSupportedDateTime.Ticks;
_maxSupportedTicks = cal.MaxSupportedDateTime.Ticks;
}

// EraInfo.yearOffset: The offset to Gregorian year when the era starts. Gregorian Year = Era Year + yearOffset
Expand Down Expand Up @@ -168,7 +172,9 @@ internal bool IsValidYear(int year, int era)

internal void CheckTicksRange(long ticks)
{
if (ticks < m_Cal.MinSupportedDateTime.Ticks || ticks > m_Cal.MaxSupportedDateTime.Ticks)
if (ticks < _minSupportedTicks || ticks > _maxSupportedTicks) ThrowOutOfRange();

void ThrowOutOfRange()
{
throw new ArgumentOutOfRangeException(
"time",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,9 @@ public static NumberFormatInfo GetInstance(IFormatProvider? formatProvider)
static NumberFormatInfo GetProviderNonNull(IFormatProvider provider)
{
// Fast path for a regular CultureInfo
if (provider is CultureInfo cultureProvider && !cultureProvider._isInherited)
if (provider.GetType() == typeof(CultureInfo) && ((CultureInfo)provider)._numInfo is { } info)
{
return cultureProvider._numInfo ?? cultureProvider.NumberFormat;
return info;
}

return
Expand Down
9 changes: 6 additions & 3 deletions src/libraries/System.Private.CoreLib/src/System/IO/File.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,12 @@ public static SafeFileHandle OpenHandle(string path, FileMode mode = FileMode.Op
// File and Directory UTC APIs treat a DateTimeKind.Unspecified as UTC whereas
// ToUniversalTime treats this as local.
internal static DateTimeOffset GetUtcDateTimeOffset(DateTime dateTime)
=> dateTime.Kind == DateTimeKind.Unspecified
? DateTime.SpecifyKind(dateTime, DateTimeKind.Utc)
: dateTime.ToUniversalTime();
{
if (dateTime.Kind == DateTimeKind.Local)
dateTime = dateTime.ToUniversalTime();

return new DateTimeOffset(dateTime.Ticks, default);
}

public static void SetCreationTime(string path, DateTime creationTime)
=> FileSystem.SetCreationTime(Path.GetFullPath(path), creationTime, asDirectory: false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,30 +466,24 @@ private static void SetFileTime(
bool asDirectory,
long creationTime = 0,
long lastAccessTime = 0,
long lastWriteTime = 0,
long changeTime = 0,
uint fileAttributes = 0)
long lastWriteTime = 0)
{
using SafeFileHandle handle = OpenHandleToWriteAttributes(fullPath, asDirectory);
SetFileTime(handle, fullPath, creationTime, lastAccessTime, lastWriteTime, changeTime, fileAttributes);
SetFileTime(handle, fullPath, creationTime, lastAccessTime, lastWriteTime);
}

private static unsafe void SetFileTime(
SafeFileHandle fileHandle,
string? fullPath = null,
long creationTime = 0,
long lastAccessTime = 0,
long lastWriteTime = 0,
long changeTime = 0,
uint fileAttributes = 0)
long lastWriteTime = 0)
{
var basicInfo = new Interop.Kernel32.FILE_BASIC_INFO
{
CreationTime = creationTime,
LastAccessTime = lastAccessTime,
LastWriteTime = lastWriteTime,
ChangeTime = changeTime,
FileAttributes = fileAttributes
};

if (!Interop.Kernel32.SetFileInformationByHandle(fileHandle, Interop.Kernel32.FileBasicInfo, &basicInfo, (uint)sizeof(Interop.Kernel32.FILE_BASIC_INFO)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -535,8 +535,7 @@ private TransitionTime GetNextTransitionTimeValue()

TransitionTime transition;

DateTime timeOfDay = GetNextDateTimeValue(TimeOfDayFormat);
timeOfDay = new DateTime(1, 1, 1, timeOfDay.Hour, timeOfDay.Minute, timeOfDay.Second, timeOfDay.Millisecond);
DateTime timeOfDay = TimeOnly.FromDateTime(GetNextDateTimeValue(TimeOfDayFormat)).ToDateTime();

int month = GetNextInt32Value();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ private static void ValidateTransitionTime(DateTime timeOfDay, int month, int we
throw new ArgumentOutOfRangeException(nameof(dayOfWeek), SR.ArgumentOutOfRange_DayOfWeek);
}

timeOfDay.GetDate(out int timeOfDayYear, out int timeOfDayMonth, out int timeOfDayDay);
if (timeOfDayYear != 1 || timeOfDayMonth != 1 || timeOfDayDay != 1 || (timeOfDay.Ticks % TimeSpan.TicksPerMillisecond != 0))
if (timeOfDay.Ticks >= TimeSpan.TicksPerDay || (ulong)timeOfDay.Ticks % TimeSpan.TicksPerMillisecond != 0)
{
throw new ArgumentException(SR.Argument_DateTimeHasTicks, nameof(timeOfDay));
}
Expand Down
44 changes: 20 additions & 24 deletions src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -711,8 +711,8 @@ public static DateTimeOffset ConvertTime(DateTimeOffset dateTimeOffset, TimeZone
long ticks = utcDateTime.Ticks + destinationOffset.Ticks;

return
ticks > DateTimeOffset.MaxValue.Ticks ? DateTimeOffset.MaxValue :
ticks < DateTimeOffset.MinValue.Ticks ? DateTimeOffset.MinValue :
ticks > DateTime.MaxTicks ? DateTimeOffset.MaxValue :
ticks < DateTime.MinTicks ? DateTimeOffset.MinValue :
new DateTimeOffset(ticks, destinationOffset);
}

Expand Down Expand Up @@ -836,10 +836,6 @@ public static DateTime ConvertTimeToUtc(DateTime dateTime)
/// </summary>
internal static DateTime ConvertTimeToUtc(DateTime dateTime, TimeZoneInfoOptions flags)
{
pentp marked this conversation as resolved.
Show resolved Hide resolved
if (dateTime.Kind == DateTimeKind.Utc)
{
return dateTime;
}
CachedData cachedData = s_cachedData;
return ConvertTime(dateTime, cachedData.Local, s_utcTimeZone, flags, cachedData);
}
Expand Down Expand Up @@ -1298,8 +1294,8 @@ private DateTime ConvertToFromUtc(DateTime dateTime, TimeSpan daylightDelta, Tim
long ticks = dateTime.Ticks + offset.Ticks;

return
ticks > DateTime.MaxValue.Ticks ? DateTime.MaxValue :
ticks < DateTime.MinValue.Ticks ? DateTime.MinValue :
ticks > DateTime.MaxTicks ? DateTime.MaxValue :
ticks < DateTime.MinTicks ? DateTime.MinValue :
new DateTime(ticks);
}

Expand All @@ -1312,17 +1308,17 @@ private static DateTime ConvertUtcToTimeZone(long ticks, TimeZoneInfo destinatio
{
// used to calculate the UTC offset in the destinationTimeZone
DateTime utcConverted =
ticks > DateTime.MaxValue.Ticks ? DateTime.MaxValue :
ticks < DateTime.MinValue.Ticks ? DateTime.MinValue :
ticks > DateTime.MaxTicks ? DateTime.MaxValue :
ticks < DateTime.MinTicks ? DateTime.MinValue :
new DateTime(ticks);

// verify the time is between MinValue and MaxValue in the new time zone
TimeSpan offset = GetUtcOffsetFromUtc(utcConverted, destinationTimeZone, out isAmbiguousLocalDst);
ticks += offset.Ticks;

return
ticks > DateTime.MaxValue.Ticks ? DateTime.MaxValue :
ticks < DateTime.MinValue.Ticks ? DateTime.MinValue :
ticks > DateTime.MaxTicks ? DateTime.MaxValue :
ticks < DateTime.MinTicks ? DateTime.MinValue :
new DateTime(ticks);
}

Expand Down Expand Up @@ -1373,11 +1369,11 @@ private static bool GetIsDaylightSavings(DateTime time, AdjustmentRule rule, Day
// startTime and endTime represent the period from either the start of
// DST to the end and ***includes*** the potentially overlapped times
startTime = rule.IsStartDateMarkerForBeginningOfYear() ?
new DateTime(daylightTime.Start.Year, 1, 1, 0, 0, 0) :
new DateTime(daylightTime.Start.Year, 1, 1) :
daylightTime.Start + daylightTime.Delta;

endTime = rule.IsEndDateMarkerForEndOfYear() ?
new DateTime(daylightTime.End.Year + 1, 1, 1, 0, 0, 0).AddTicks(-1) :
new DateTime(daylightTime.End.Year + 1, 1, 1).AddTicks(-1) :
daylightTime.End;
}
else
Expand All @@ -1402,11 +1398,11 @@ private static bool GetIsDaylightSavings(DateTime time, AdjustmentRule rule, Day
bool invalidAtStart = rule.DaylightDelta > TimeSpan.Zero;

startTime = rule.IsStartDateMarkerForBeginningOfYear() ?
new DateTime(daylightTime.Start.Year, 1, 1, 0, 0, 0) :
new DateTime(daylightTime.Start.Year, 1, 1) :
daylightTime.Start + (invalidAtStart ? rule.DaylightDelta : TimeSpan.Zero); /* FUTURE: - rule.StandardDelta; */

endTime = rule.IsEndDateMarkerForEndOfYear() ?
new DateTime(daylightTime.End.Year + 1, 1, 1, 0, 0, 0).AddTicks(-1) :
new DateTime(daylightTime.End.Year + 1, 1, 1).AddTicks(-1) :
daylightTime.End + (invalidAtStart ? -rule.DaylightDelta : TimeSpan.Zero);
}

Expand Down Expand Up @@ -1484,15 +1480,15 @@ private static bool GetIsDaylightSavingsFromUtc(DateTime time, int year, TimeSpa
bool ignoreYearAdjustment = false;
TimeSpan dstStartOffset = zone.GetDaylightSavingsStartOffsetFromUtc(utc, rule, ruleIndex);
DateTime startTime;
if (rule.IsStartDateMarkerForBeginningOfYear() && daylightTime.Start.Year > DateTime.MinValue.Year)
if (rule.IsStartDateMarkerForBeginningOfYear() && daylightTime.Start.Year is > 1 and int startYear)
{
if (TryGetStartOfDstIfYearEndWithDst(daylightTime.Start.Year - 1, utc, zone, out startTime))
if (TryGetStartOfDstIfYearEndWithDst(startYear - 1, utc, zone, out startTime))
{
ignoreYearAdjustment = true;
}
else
{
startTime = new DateTime(daylightTime.Start.Year, 1, 1, 0, 0, 0) - dstStartOffset;
startTime = new DateTime(startYear, 1, 1) - dstStartOffset;
}
}
else
Expand All @@ -1502,15 +1498,15 @@ private static bool GetIsDaylightSavingsFromUtc(DateTime time, int year, TimeSpa

TimeSpan dstEndOffset = GetDaylightSavingsEndOffsetFromUtc(utc, rule);
DateTime endTime;
if (rule.IsEndDateMarkerForEndOfYear() && daylightTime.End.Year < DateTime.MaxValue.Year)
if (rule.IsEndDateMarkerForEndOfYear() && daylightTime.End.Year is < 9999 and int endYear)
{
if (TryGetEndOfDstIfYearStartWithDst(daylightTime.End.Year + 1, utc, zone, out endTime))
if (TryGetEndOfDstIfYearStartWithDst(endYear + 1, utc, zone, out endTime))
{
ignoreYearAdjustment = true;
}
else
{
endTime = new DateTime(daylightTime.End.Year + 1, 1, 1, 0, 0, 0).AddTicks(-1) - dstEndOffset;
endTime = new DateTime(endYear + 1, 1, 1).AddTicks(-1) - dstEndOffset;
}
}
else
Expand Down Expand Up @@ -2190,8 +2186,8 @@ private static void ValidateTimeZoneInfo(string id, TimeSpan baseUtcOffset, Adju
}
}

private static readonly TimeSpan MaxOffset = TimeSpan.FromHours(14.0);
private static readonly TimeSpan MinOffset = -MaxOffset;
private static TimeSpan MaxOffset => TimeSpan.FromHours(14);
private static TimeSpan MinOffset => TimeSpan.FromHours(-14);
tarekgh marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Helper function that validates the TimeSpan is within +/- 14.0 hours
Expand Down