Skip to content

Enhance the date and time precision in Add methods #73198

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.

Sign up for GitHub

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

Merged
tarekgh merged 3 commits into from
Aug 4, 2022
Merged

Enhance the date and time precision in Add methods #73198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45f13a0
Enhance the date and time precision in Add methods
tarekgh Jul 30, 2022
63d7538
Refactoring and fixing a test
tarekgh Aug 2, 2022
198ac12
Remove un-needed space
tarekgh Aug 2, 2022
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 64 additions & 73 deletions src/libraries/System.Private.CoreLib/src/System/DateTime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,17 @@ public readonly partial struct DateTime
private const long TicksPerMicrosecond = 10;
private const long TicksPerMillisecond = TicksPerMicrosecond * MicrosecondsPerMillisecond;

private const int HoursPerDay = 24;
private const long TicksPerSecond = TicksPerMillisecond * 1000;
private const long TicksPerMinute = TicksPerSecond * 60;
private const long TicksPerHour = TicksPerMinute * 60;
private const long TicksPerDay = TicksPerHour * 24;
private const long TicksPerDay = TicksPerHour * HoursPerDay;

// Number of milliseconds per time unit
private const int MillisPerSecond = 1000;
private const int MillisPerMinute = MillisPerSecond * 60;
private const int MillisPerHour = MillisPerMinute * 60;
private const int MillisPerDay = MillisPerHour * 24;
private const int MillisPerDay = MillisPerHour * HoursPerDay;

// Number of days in a non-leap year
private const int DaysPerYear = 365;
Expand All @@ -90,8 +91,12 @@ public readonly partial struct DateTime

internal const long MinTicks = 0;
internal const long MaxTicks = DaysTo10000 * TicksPerDay - 1;
private const long MaxMillis = (long)DaysTo10000 * MillisPerDay;
private const long MaxMicroseconds = MaxMillis * MicrosecondsPerMillisecond;
private const long MaxMicroseconds = MaxTicks / TicksPerMicrosecond;
private const long MaxMillis = MaxTicks / TicksPerMillisecond;
private const long MaxSeconds = MaxTicks / TicksPerSecond;
private const long MaxMinutes = MaxTicks / TicksPerMinute;
private const long MaxHours = MaxTicks / TicksPerHour;
private const long MaxDays = (long)DaysTo10000 - 1;

internal const long UnixEpochTicks = DaysTo1970 * TicksPerDay;
private const long FileTimeOffset = DaysTo1601 * TicksPerDay;
Expand Down Expand Up @@ -178,6 +183,7 @@ internal DateTime(long ticks, DateTimeKind kind, bool isAmbiguousDst)
private static void ThrowMillisecondOutOfRange() => throw new ArgumentOutOfRangeException("millisecond", SR.Format(SR.ArgumentOutOfRange_Range, 0, MillisPerSecond - 1));
private static void ThrowMicrosecondOutOfRange() => throw new ArgumentOutOfRangeException("microsecond", SR.Format(SR.ArgumentOutOfRange_Range, 0, MicrosecondsPerMillisecond - 1));
private static void ThrowDateArithmetic(int param) => throw new ArgumentOutOfRangeException(param switch { 0 => "value", 1 => "t", _ => "months" }, SR.ArgumentOutOfRange_DateArithmetic);
private static void ThrowAddOutOfRange() => throw new ArgumentOutOfRangeException("value", SR.ArgumentOutOfRange_AddValue);

// Constructs a DateTime from a given year, month, and day. The
// time-of-day of the resulting DateTime is always midnight.
Expand Down Expand Up @@ -835,49 +841,48 @@ public DateTime Add(TimeSpan value)
return AddTicks(value._ticks);
}

// Returns the DateTime resulting from adding a fractional number of
// time units to this DateTime.
private DateTime Add(double value, int scale)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private DateTime AddUnits(double value, long maxUnitCount, long ticksPerUnit)
{
double millis_double = value * scale + (value >= 0 ? 0.5 : -0.5);
if (millis_double <= -MaxMillis || millis_double >= MaxMillis) ThrowOutOfRange();
return AddTicks((long)millis_double * TicksPerMillisecond);
if (Math.Abs(value) > maxUnitCount)
{
ThrowAddOutOfRange();
}

static void ThrowOutOfRange() => throw new ArgumentOutOfRangeException(nameof(value), SR.ArgumentOutOfRange_AddValue);
}
double integralPart = Math.Truncate(value);
double fractionalPart = value - integralPart;
long ticks = (long)(integralPart) * ticksPerUnit;
ticks += (long)(fractionalPart * ticksPerUnit);

// Returns the DateTime resulting from adding a fractional number of
// days to this DateTime. The result is computed by rounding the
// fractional number of days given by value to the nearest
// millisecond, and adding that interval to this DateTime. The
// value argument is permitted to be negative.
//
public DateTime AddDays(double value)
{
return Add(value, MillisPerDay);
return AddTicks(ticks);

@mattjohnsonpint mattjohnsonpint Aug 2, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm curious why this is needed, vs AddTicks((long) (value * TicksPerDay))

Is there a perf benefit or something? Or some edge case with floating-point math?

@tarekgh tarekgh Aug 2, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is the later some edge case with floating-point math. Can have precision loss with some values. @tannergooding can explain more details about that.

@mattjohnsonpint mattjohnsonpint Aug 2, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interesting. If that's true, then I guess that still DateTime.AddUnits(n) != DateTime.Add(TimeSpan.FromUnits(n)) for some units and some value of n?

@tarekgh tarekgh Aug 2, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, looks so. This can happen today too, I guess. We can track fixing in TimeSpan later. I wanted to get this fix now for .NET 7.0 and then we can fix any more issues as needed in the next releases.

@tannergooding tannergooding Aug 2, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right. double can only represent integrals exactly up to 2^53 anything higher than this will definitely have rounding error. Since there are approximately 31,540,000,000,000,000 nanoseconds per year, this is approximately 315,400,000,000,000 ticks per year. 2^53 / TicksPerYear gives you approximately 28.54 years (or 10,425 days) before definite rounding error starts creeping in.

This ends up with two very visible edge-case considerations:

  1. If the user inputs a double value that is already greater than 2^53 there is guaranteed loss of precision
  2. If the user inputs a double value where (value * TicksPerUnit) > 2^53 there is guaranteed loss of precision

There is then a third, but less visible, consideration that these two rules applies mainly to the integral portion. The fractional portion is much tricker to understand since almost anything the user inputs has some loss of data due to double only being able to represent multiples of power of 2.

What this means is that at 2^52 to 2^53, double can represent no fractional data (that is it can only represent multiples of 1). At 2^51 to 2^52, double can only represent multiples of 0.5, at 2^50 to 2^51, double can only represent multiples of 0.25, then 0.125, then 0.0625, and so on doubling in precision every smaller "power of two" down to double.Epsilon at Zero. This likewise halves the precision every larger "power of two" (2^53 to 2^54 can only represent multiples of 2, then 4, then 8, and so on).

This boils down to: the closer (value * TicksPerUnit) is to 2^53, the more loss of data compared to the input of value you will observe. Splitting it into an integral and fractional portion helps reduce the overall rounding error by ensuring that the integral portion is handled and then the fractional portion by itself, which allows the most accuracy when it is scaled up.

=====================

The current algorithm (which is (long)((value * MillisPerUnit) + Adjustment) * MillisecondsPerTick) tries to minimize rounding error by computing a double that is MillisPerUnit rather than one that is TicksPerUnit. This attempts to guarantee millisecond accuracy (but not microsecond or nanosecond) and broadens the range significantly to some 28k years.

Replacing the algorithm with purely x * TicksPerDay will end up broaching 2^53 much sooner and will cause the fractional part to no longer be considered (resulting in the result being off in various edge cases).

A purely correct, tick accurate, approach would be more complex and likely cost too much for perf compared to the current algorithm.

However, splitting it into integer and fractional portions will likewise end up broaching 2^53 much sooner, but will in turn allow for the fractional part to be correctly considered. It likewise maintains millisecond accuracy. This is because, given the upper bound of integer accuracy: 9,007,199,254,740,992 millisecond accuracy involves ignoring the lowest 4 digits: 9,007,199,254,740,000 and we have a maximum error of only 1024 ticks at 2^63. -- This is within 102.4 microseconds which means that we can get up to tick accuracy, but no worse than millisecond accuracy.

=====================

As an aside, when considering #66815, we must first consider that var seconds = 0.9999999; is actually seconds = 0.99999990000000005263558477963670156896114349365234375 (as that is the nearest representable double to 0.9999999), it's not going to impact this scenario much but it is important to consider that values aren't always as "exact" as it might appear and so results may be "off" in other contexts regardless.
}

// Returns the DateTime resulting from adding a fractional number of
// hours to this DateTime. The result is computed by rounding the
// fractional number of hours given by value to the nearest
// millisecond, and adding that interval to this DateTime. The
// value argument is permitted to be negative.
//
public DateTime AddHours(double value)
{
return Add(value, MillisPerHour);
}
/// <summary>
/// Returns a new <see cref="DateTime"/> that adds the specified number of days to the value of this instance.
/// </summary>
/// <param name="value">A number of whole and fractional days. The value parameter can be negative or positive.</param>
/// <returns>
/// An object whose value is the sum of the date and time represented by this instance and the number of days represented by value.
/// </returns>
public DateTime AddDays(double value) => AddUnits(value, MaxDays, TicksPerDay);

// Returns the DateTime resulting from the given number of
// milliseconds to this DateTime. The result is computed by rounding
// the number of milliseconds given by value to the nearest integer,
// and adding that interval to this DateTime. The value
// argument is permitted to be negative.
//
public DateTime AddMilliseconds(double value)
{
return Add(value, 1);
}
/// <summary>
/// Returns a new <see cref="DateTime"/> that adds the specified number of hours to the value of this instance.
/// </summary>
/// <param name="value">A number of whole and fractional hours. The value parameter can be negative or positive.</param>
/// <returns>
/// An object whose value is the sum of the date and time represented by this instance and the number of hours represented by value.
/// </returns>
public DateTime AddHours(double value) => AddUnits(value, MaxHours, TicksPerHour);

/// <summary>
/// Returns a new <see cref="DateTime"/> that adds the specified number of milliseconds to the value of this instance.
/// </summary>
/// <param name="value">A number of whole and fractional milliseconds. The value parameter can be negative or positive.</param>
/// <returns>
/// An object whose value is the sum of the date and time represented by this instance and the number of milliseconds represented by value.
/// </returns>
public DateTime AddMilliseconds(double value) => AddUnits(value, MaxMillis, TicksPerMillisecond);

/// <summary>
/// Returns a new <see cref="DateTime"/> that adds the specified number of microseconds to the value of this instance.
Expand All @@ -903,28 +908,16 @@ public DateTime AddMilliseconds(double value)
/// <exception cref="ArgumentOutOfRangeException">
/// The resulting <see cref="DateTime"/> is less than <see cref="MinValue"/> or greater than <see cref="MaxValue"/>.
/// </exception>
public DateTime AddMicroseconds(double value)
{
if (value < -MaxMicroseconds || value > MaxMicroseconds)
{
ThrowOutOfRange();
}

return AddTicks((long)(value * TicksPerMicrosecond));
public DateTime AddMicroseconds(double value) => AddUnits(value, MaxMicroseconds, TicksPerMicrosecond);

static void ThrowOutOfRange() => throw new ArgumentOutOfRangeException(nameof(value), SR.ArgumentOutOfRange_AddValue);
}

// Returns the DateTime resulting from adding a fractional number of
// minutes to this DateTime. The result is computed by rounding the
// fractional number of minutes given by value to the nearest
// millisecond, and adding that interval to this DateTime. The
// value argument is permitted to be negative.
//
public DateTime AddMinutes(double value)
{
return Add(value, MillisPerMinute);
}
/// <summary>
/// Returns a new <see cref="DateTime"/> that adds the specified number of minutes to the value of this instance.
/// </summary>
/// <param name="value">A number of whole and fractional minutes. The value parameter can be negative or positive.</param>
/// <returns>
/// An object whose value is the sum of the date and time represented by this instance and the number of minutes represented by value.
/// </returns>
public DateTime AddMinutes(double value) => AddUnits(value, MaxMinutes, TicksPerMinute);

// Returns the DateTime resulting from adding the given number of
// months to this DateTime. The result is computed by incrementing
Expand Down Expand Up @@ -961,16 +954,14 @@ public DateTime AddMonths(int months)
return new DateTime(n * (ulong)TicksPerDay + UTicks % TicksPerDay | InternalKind);
}

// Returns the DateTime resulting from adding a fractional number of
// seconds to this DateTime. The result is computed by rounding the
// fractional number of seconds given by value to the nearest
// millisecond, and adding that interval to this DateTime. The
// value argument is permitted to be negative.
//
public DateTime AddSeconds(double value)
{
return Add(value, MillisPerSecond);
}
/// <summary>
/// Returns a new <see cref="DateTime"/> that adds the specified number of seconds to the value of this instance.
/// </summary>
/// <param name="value">A number of whole and fractional seconds. The value parameter can be negative or positive.</param>
/// <returns>
/// An object whose value is the sum of the date and time represented by this instance and the number of seconds represented by value.
/// </returns>
public DateTime AddSeconds(double value) => AddUnits(value, MaxSeconds, TicksPerSecond);

// Returns the DateTime resulting from adding the given number of
// 100-nanosecond ticks to this DateTime. The value argument
Expand Down Expand Up @@ -1161,7 +1152,7 @@ internal static long DoubleDateToTicks(double value)

millis += DoubleDateOffset / TicksPerMillisecond;

if (millis < 0 || millis >= MaxMillis) throw new ArgumentException(SR.Arg_OleAutDateScale);
if (millis < 0 || millis > MaxMillis) throw new ArgumentException(SR.Arg_OleAutDateScale);
return millis * TicksPerMillisecond;
}

Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Private.Xml/tests/XmlConvert/ToTypeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ public int ToType52()
var param = (string)CurVariation.Param;

object[] array0 = { "2002-12-30", "23:15:55", "2002-01-09T04:02:08", "2002-01-09T04:02:08Z", "2002-01-09Z", "2002-01-09T04:02:08-05:00", "0002-01", "2016-02-29", "9999", "9999Z", "9999-12-31T12:59:59+14:00", "9999-12-31T12:59:59-11:00", "9999-12-31T12:59:59-10:59", "9999-12-31T12:59:59+13:59", "9999-12-31T23:59:59-00:00", "9999-12-31T23:59:59+14:00", "9998-12-31T12:59:59+14:00", "9998-12-31T12:59:59-14:00", "0002", "0001Z", "0002-01-01T00:00:00-14:00", "0002-01-01T00:00:00-13:59", "0002-01-01T00:00:00+00:00", "0002-01-01T00:00:00-00:00", "2008-02-29T23:59:59-14:00", "2012-02-29T23:59:59+14:00" };
object[] array1 = { new DateTimeOffset(2002, 12, 30, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2002, 12, 30))), new DateTimeOffset(DateTime.Now.Year, DateTime.Now.Month, DateTime.Now.Day, 23, 15, 55, TimeZoneInfo.Local.GetUtcOffset(new DateTime(DateTime.Now.Year, DateTime.Now.Month, DateTime.Now.Day, 23, 15, 55))), (new DateTimeOffset(2002, 1, 9, 4, 2, 8, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2002, 1, 9)))).AddMilliseconds(0.1458925435), (new DateTimeOffset(2002, 1, 9, 4, 2, 8, TimeSpan.FromHours(0))).ToLocalTime(), (new DateTimeOffset(2002, 1, 9, 0, 0, 0, TimeSpan.FromHours(0))).ToLocalTime(), (new DateTimeOffset(2002, 1, 9, 4, 2, 8, new TimeSpan(-5, 0, 0))), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2, 1, 1))), new DateTimeOffset(2016, 2, 29, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2016, 2, 29))), new DateTimeOffset(9999, 1, 1, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(9999, 1, 1))), new DateTimeOffset(9999, 1, 1, 0, 0, 0, TimeSpan.FromHours(0)), new DateTimeOffset(9999, 12, 31, 12, 59, 59, TimeSpan.FromHours(14.0)), new DateTimeOffset(9999, 12, 31, 12, 59, 59, TimeSpan.FromHours(-11.0)), new DateTimeOffset(9999, 12, 31, 12, 59, 59, TimeSpan.FromHours(-10) + TimeSpan.FromMinutes(-59)), new DateTimeOffset(9999, 12, 31, 12, 59, 59, new TimeSpan(13, 59, 0)), new DateTimeOffset(9999, 12, 31, 23, 59, 59, TimeSpan.Zero), new DateTimeOffset(9999, 12, 31, 23, 59, 59, TimeSpan.FromHours(14.0)), new DateTimeOffset(9998, 12, 31, 12, 59, 59, TimeSpan.FromHours(14.0)), new DateTimeOffset(9998, 12, 31, 12, 59, 59, TimeSpan.FromHours(-14.0)), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2, 1, 1))), new DateTimeOffset(1, 1, 1, 0, 0, 0, TimeSpan.FromHours(0)), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeSpan.FromHours(-14.0)), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeSpan.FromHours(-13) + TimeSpan.FromMinutes(-59)), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeSpan.Zero), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeSpan.Zero), new DateTimeOffset(2008, 2, 29, 23, 59, 59, TimeSpan.FromHours(-14)), new DateTimeOffset(2012, 2, 29, 23, 59, 59, TimeSpan.FromHours(14)) };
object[] array1 = { new DateTimeOffset(2002, 12, 30, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2002, 12, 30))), new DateTimeOffset(DateTime.Now.Year, DateTime.Now.Month, DateTime.Now.Day, 23, 15, 55, TimeZoneInfo.Local.GetUtcOffset(new DateTime(DateTime.Now.Year, DateTime.Now.Month, DateTime.Now.Day, 23, 15, 55))), (new DateTimeOffset(2002, 1, 9, 4, 2, 8, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2002, 1, 9)))), (new DateTimeOffset(2002, 1, 9, 4, 2, 8, TimeSpan.FromHours(0))).ToLocalTime(), (new DateTimeOffset(2002, 1, 9, 0, 0, 0, TimeSpan.FromHours(0))).ToLocalTime(), (new DateTimeOffset(2002, 1, 9, 4, 2, 8, new TimeSpan(-5, 0, 0))), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2, 1, 1))), new DateTimeOffset(2016, 2, 29, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2016, 2, 29))), new DateTimeOffset(9999, 1, 1, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(9999, 1, 1))), new DateTimeOffset(9999, 1, 1, 0, 0, 0, TimeSpan.FromHours(0)), new DateTimeOffset(9999, 12, 31, 12, 59, 59, TimeSpan.FromHours(14.0)), new DateTimeOffset(9999, 12, 31, 12, 59, 59, TimeSpan.FromHours(-11.0)), new DateTimeOffset(9999, 12, 31, 12, 59, 59, TimeSpan.FromHours(-10) + TimeSpan.FromMinutes(-59)), new DateTimeOffset(9999, 12, 31, 12, 59, 59, new TimeSpan(13, 59, 0)), new DateTimeOffset(9999, 12, 31, 23, 59, 59, TimeSpan.Zero), new DateTimeOffset(9999, 12, 31, 23, 59, 59, TimeSpan.FromHours(14.0)), new DateTimeOffset(9998, 12, 31, 12, 59, 59, TimeSpan.FromHours(14.0)), new DateTimeOffset(9998, 12, 31, 12, 59, 59, TimeSpan.FromHours(-14.0)), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeZoneInfo.Local.GetUtcOffset(new DateTime(2, 1, 1))), new DateTimeOffset(1, 1, 1, 0, 0, 0, TimeSpan.FromHours(0)), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeSpan.FromHours(-14.0)), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeSpan.FromHours(-13) + TimeSpan.FromMinutes(-59)), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeSpan.Zero), new DateTimeOffset(2, 1, 1, 0, 0, 0, TimeSpan.Zero), new DateTimeOffset(2008, 2, 29, 23, 59, 59, TimeSpan.FromHours(-14)), new DateTimeOffset(2012, 2, 29, 23, 59, 59, TimeSpan.FromHours(14)) };
string[] format = { "yyyy-MM-dd", "HH:mm:ss", "yyyy-MM-ddTHH:mm:ss", "yyyy-MM-ddTHH:mm:ssZ", "yyyy-MM-ddZ", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM", "yyyy-MM-dd", "yyyy", "yyyyZ", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy", "yyyyZ", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz", "yyyy-MM-ddTHH:mm:sszzzzzz" };
return TestValid(array0, array1, param, format);
}
Expand Down
Loading