Optimize DateOnly, TimeOnly, ISOWeek #111244
Conversation
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-datetime |
| @@ -1086,7 +1086,7 @@ private static ulong DateToTicks(int year, int month, int day) | |||
| ThrowHelper.ThrowArgumentOutOfRange_BadYearMonthDay(); | |||
| } | |||
|
|
|||
| ReadOnlySpan<uint> days = IsLeapYear(year) ? DaysToMonth366 : DaysToMonth365; | |||
| ReadOnlySpan<uint> days = month > 1 && IsLeapYear(year) ? DaysToMonth366 : DaysToMonth365; | |||
There was a problem hiding this comment.
I understand it is to avoid calling IsLeapYear. But wouldn't this check add cost when having other values of month?
There was a problem hiding this comment.
The main benefit is actually for the quite common case where a DateTime is constructed with const month=1,day=1 - this change allows skipping not only the leap year check but also the DaysToMonth lookup and check. The overhead for non-const cases is just one branch.
There was a problem hiding this comment.
the quite common case where a DateTime is constructed with const month=1,day=1
I am not sure this is true. I rarely see DateTime constructed with that.
|
|
||
| /// <summary> | ||
| /// Gets the minute component of the time represented by this instance. | ||
| /// </summary> | ||
| public int Minute => new TimeSpan(_ticks).Minutes; | ||
| public int Minute => (int)((uint)(_ticks / TimeSpan.TicksPerMinute) % 60); |
|
|
||
| /// <summary> | ||
| /// Gets the second component of the time represented by this instance. | ||
| /// </summary> | ||
| public int Second => new TimeSpan(_ticks).Seconds; | ||
| public int Second => (int)((uint)(_ticks / TimeSpan.TicksPerSecond) % 60); |
There was a problem hiding this comment.
TimeSpan supports large ranges, including negative values. TimeOnly values are more constrained.
There was a problem hiding this comment.
Yes, I understand that. I was wondering if it is worth the minor perf gain to repeat the calculation here.
| public static TimeSpan operator -(TimeOnly t1, TimeOnly t2) | ||
| { | ||
| long diff = (long)(t1._ticks - t2._ticks); | ||
| return new TimeSpan(diff + ((diff >> 63) & TimeSpan.TicksPerDay)); |
src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeFormat.cs
Show resolved
Hide resolved
|
Any benchmark numbers for the optimized parts? |
|
Maybe rename the method |
Small optimizations and cleanup.