dotnet / dotnet-api-docs

.NET API reference documentation (.NET 5+, .NET Core, .NET Framework)
https://docs.microsoft.com/dotnet/api/
Other
705 stars 1.55k forks source link

Leap second statement is confusing #966

Open perennialmind opened 5 years ago

perennialmind commented 5 years ago

At the time of writing, the documentation reads

represents the number of seconds that have elapsed

and

does not take leap seconds into account

I found this wording confusing. At first reading, I questioned whether the calculation treated leap seconds differently from absolute monotonic seconds, as it should. Based on cursory tests, I can see that calculations are correctly performed in the calendar domain (UTC), not the time domain (TAI). The best wording I have seen to date is this, by Markus Kuhn:

The POSIX second count ignores inserted leap seconds (and does not even provide an encoding for them) and counts deleted leap seconds

In practice, unix time represents a UTC date and time as a set of fields packed into an integer value according to a formula set by POSIX. The companion definition of unix time as a measure of elapsed time cannot be reconciled with this formula; it is effectively a false correspondence rooted in history and is a common source of error. It appears that ToUnixTimeSeconds uses the latter definition and conforms because the DateTimeOffset.Ticks shares the same limitation regarding leap seconds. The documentation there is more clear:

It does not include ticks that would be added by leap seconds.

(edited to fix blockquote)

Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

Thraka commented 5 years ago

@perennialmind Hi!

Are you saying that the "does not take leap seconds into account" should be a bit more clear about where those seconds come from, as the ToUnixTimeSeconds method explains it?

perennialmind commented 5 years ago

The wording of the documentation for DateTimeOffset.Ticks is short, clear, and precise. Ticks and UnixTime are aligned conceptually and by implementation, so using the same wording makes sense to me. The other wording I quoted is also clear and precise, with additional information for context. Either would be an improvement.

Knowing that leap seconds are a potential source of inaccuracy is important. If the contract is narrowed, the platform interface may be reliably combined with external data (a leap second table) to make faithful calculations in both true elapsed time and calendar time.

perennialmind commented 5 years ago

@Thraka, I think I may have missed your point. I would not expect the documentation to define leap seconds, just how they are handled. I had to review the code to find out. (The specs take leap seconds into account insofar as they are explicitly excluded from the count). The implementation is free to ignore them because Windows date/time functions have historically ignored them; that's changing, and I hope to know how that will impact DateTime. That's where the docs come in :-)

It might even be worth calling out the behavior during a leap second. I gather that the latest Windows release will report second '59' for two seconds in GetSystemTime for processes that don't opt-in to leap second support. Does GetSystemTimeAsFileTime follow suit? Hold constant for a full second, as a literal reading of the docs would imply? I don't know, but it might be worth a few words, even if only to say the behavior is "system dependent".

tarekgh commented 5 years ago

@perennialmind here is some clarification how the .NET (version 4.7.2) work on the version of Windows that support the leap seconds (i.e. Windows 10 RS5 release):

Let me know if you have any more questions or you need more clarification

Thraka commented 5 years ago

@tarekgh Thank you for the great explanation. Would you be interested in updating the docs to make it clearer?

tarekgh commented 5 years ago

I would wait a little if there is any change will happen in the near future.

CC @rpetrusha our technical writer who I think can help with the docs.

rpetrusha commented 5 years ago

I hope you don't mind, @Thraka if I assign this issue to myself. I just met with @tarekgh to discuss the documentation needed for leap seconds.

perennialmind commented 5 years ago

@tarekgh, thank you! I'm sorry for taking so long to respond.

So Ticks will stay on the original windows "timeline", using the terminology from Dan Cuomo's "Leap Seconds for the IT Pro: What you need to know" blog entry – whether or not the ProcessLeapSecondInfo flag is in effect. That strikes me as generally desirable.

I went looking through the 4.7.2 and coreclr code available to better understand the logic you described and came up empty. If I understand your description correctly, DateTime.UtcNow will:

Do I have that right?

That only leaves me with one question. When handling wSecond=60 and adjusting down to Seconds=59, is the sub-ms component still added as-is or rounded up to 999900 ns? In other words, during a leap second, do Ticks accumulate as:

▁▂▃▄▅▆▇█▁▂▃▄▅▆▇█

or

▁▂▃▄▅▆▇█████████

?

tarekgh commented 5 years ago

So Ticks will stay on the original windows "timeline", using the terminology from Dan Cuomo's

What you mean by Ticks here? are you referring to DateTime.Ticks?

Do I have that right?

Yes, you got it right. just to be explicit, we'll also add the fractions of milliseconds (in 100-nanoseconds) to the date time ticks for the sake of the time precision.

When handling wSecond=60 and adjusting down to Seconds=59, is the sub-ms component still added as-is or rounded up to 999900 ns?

it is the later, we round it up to the max 100-nano seconds to ensure always the reported time is progressive and be in order.

perennialmind commented 5 years ago

What you mean by Ticks here? are you referring to DateTime.Ticks?

Just so. Although, in my mind, I'm already aligning "Ticks" with "classic non-leap FILETIME".

Do I have that right?

Yes, you got it right. just to be explicit, we'll also add the fractions of milliseconds (in 100-nanoseconds) to the date time ticks for the sake of the time precision.

Yup. That is clearer than my wording.

When handling wSecond=60 and adjusting down to Seconds=59, is the sub-ms component still added as-is or rounded up to 999900 ns?

it is the later, we round it up to the max 100-nano seconds to ensure always the reported time is progressive and be in order.

Clear, consistent, and comprehensible behavior. I like it. I now know just what to expect.

Now if only it were possible to smuggle out the difference between Ticks and leap-FILETIME. 😏

tarekgh commented 5 years ago

Now if only it were possible to smuggle out the difference between Ticks and leap-FILETIME.

Just to mention, DateTime.ToFileTime API will return the leap-FILETIME ticks. so at least you'll have easy way to go back and force between DateTime ticks and system ticks :-)

perennialmind commented 5 years ago

Oh that is so perfect. Perfect! Heh. Maybe I should have asked for two ponies. 😁