DateTime Concerns

[mattjohnsonpint]

In many places throughout the code the current time is retrieved by calling DateTime.Now.

For example, here are just a few of the lines I found:

return Receiver.Ask(new Get(DateTime.Now + timeout), Timeout.InfiniteTimeSpan);

var cancelable = Context.System.Scheduler.ScheduleTellOnceCancelable(next.Deadline - DateTime.Now, Self, new Kick(), Self);

This is not a good practice, as it means that code could fail during daylight saving time transitions.

DateTime.Now returns the local date and time based on the time zone setting of the computer that the code is running on, however it does not take that time zone into account when doing math.

There are few valid choices for remedies:

1. You could track all values in UTC, which does not have DST transitions. The current time would be obtained by calling DateTime.UtcNow.
2. If the local time is relevant, then you need to be working with DateTimeOffset values instead of DateTime values. The fields and properties would have to change, and the current time would come from DateTimeOffset.Now.
3. You could consider using the Noda Time library, in which case the individual values could be stored in any of Instant, OffsetDateTime, or ZonedDateTime, depending on exactly what purpose they serve.

See also the following articles on my blog:

• http://codeofmatt.com/2013/04/25/the-case-against-datetime-now/
• http://codeofmatt.com/2015/03/06/common-daylight-saving-time-mistakes-for-net-developers/

[Aaronontheweb]

I think we should just switch to UTC on those calls - since we're really only using the current time to measure elapsed time, right?

[rogeralsing]

@Aaronontheweb yepp

[mattjohnsonpint]

That would probably be the simplest thing to do to avoid DST issues.

[mattjohnsonpint]

Though - you may also want to consider what to do with existing data. If the value is persisted anywhere, then switching to UTC would mean possibly comparing the previous local data with the new UTC value. If that's something you're concerned with, the DateTimeOffset approach would solve it.

[Aaronontheweb]

@mj1856 I'll take a deeper look at the code base, but most of those calls appear to be for things that are part of the ephemeral state of an Akka.NET application as it runs in-memory. Scheduled recurring messages and so forth.

I'll look closely to see if the local clock gets accidentally exposed to the end-user anywhere they might need to store something.

But the TL;DR; of it is that I think we'll probably be ok just switching it.

[kekekeks]

If current time is used for internal scheduling purposes, anything that uses current UTC/Local time is unreliable. System time can be arbitrary changed by anything. So, it's better to use GetTickCount64 (CLOCK_MONOTONIC for *nix) which never goes backward and increments monotonically.

[kekekeks]

I've checked referencesource and Mono sources. Task.Delay uses Timer and Timer is based on monotonic clock.

[kekekeks]

Mono's Stopwatch and Environment.TickCount use CLOCK_MONOTONIC. So it's safe to just use Stopwatch every time we need to measure elapsed time. On Windows one should use GetTickCount64 for regular monotonic clock and Stopwatch for high resolution monotonic clock (Stopwatch has more overhead on Windows platform).

[HCanber]

All time related calls should go through ITimeProvider (https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka/Actor/Scheduler/ITimeProvider.cs) and not be made directly to underlying system time. Right now ITimeProvider only delivers UTC time (as a DateTimeOffset), but we should add more functions to it.
Why? Because then we can swap the ITimeProvider implementation to something else during tests,allowing us to write test that don't depend on real time, but on time that can be controlled in the test.

So for stopwatch we could add a IStopwatch CreateStopwatch(bool start=true) and rewrite @kekekeks implementation of monotonic clock (see #876) into an IStopwatch and return it from DateTimeOffsetNowTimeProvider (which probably should be renamed to something like SystemTimeProvider and let TaskBasedScheduler delegate calls to it).

[HCanber]

I seem to have mixed up two concepts here. :)
Monotonic clock should be moved into ITimeProvider as well as exposing stopwatch-functionality.

[HCanber]

Stopwatch has more overhead on Windows platform

@kekekeks Looking at your PR https://github.com/akkadotnet/akka.net/pull/876/files#diff-acc01fe0c12b1eaa67908bf90766ac05R7.
On windows, Elapsed property calls GetTickCount64 instead of Stopwatch.Elapsed (which in turn queries
the highres timer by calling QueryPerformanceCounter). So calling QueryPerformanceCounter is more expensive than GetTickCount64?

[kekekeks]

@HCanber

So calling QueryPerformanceCounter is more expensive than GetTickCount64?

See this article. TL;DR: QueryPerformanceCounter is ~88 times more expensive.

[HCanber]

We should definitely switch to use a monotonic clock instead of subtracting DateTime.Now:s, but I'm not sure exactly how, yet. We seem to have four different uses of DateTime.Now (in around 20 places):

• Get a timestamp, a long (monotonic clock ticks). See StandardMetrics in ClusterMetricsCollector.cs
• The current time, See SnapshotStore in Akka.Persistance. This should probably change to use DateTimeOffset
• Measuring intervals, i.e. the time since a point a time. See ChildRestartStats. This seems to be the most common useage of DateTime.Now
• The current time for meta data / logging / debug messages.

We have around 30 places where we use DateTime.UtcNow. I haven't looked into what it's used for, but my guess is it's used for the same reasons as for DateTime.Now.

No uses of DateTime.Today.

I'd like to think about this for a while and try different ideas I have.

[mattjohnsonpint]

@kekekeks - That article is 12 years old. Are you sure it's still accurate? I'm not sure about the perf cost on modern hardware.

Some of it is likely true still, but not sure about the cost overhead. According to this post, QueryPerformanceCounter (Stopwatch) should be used for profiling short-lived activities, while GetTickCount64 should be used for longer running operations. Still, it shouldn't be used in situations where the machine might restart, or when comparing values from multiple machines.

Still, any of those are better than using DateTime.Now, which would be prone to daylight saving time, or any of the Now or UtcNow methods, as they can be altered by user or machine adjustments to the RTC.

I've actually never used GetTickCount64 in a .NET application - but I suppose it might make sense for certain scenarios.

[kekekeks]

@mj1856

Still, it shouldn't be used in situations where the machine might restart, or when comparing values from multiple machines.

QueryPerformanceCounter accesses HPET (hardware device), GetTickCount/GetTickCount64 just reads a value already stored in the system memory that is periodically updated by OS.

Still, it shouldn't be used in situations where the machine might restart, or when comparing values from multiple machines.

It's monotonic clock. It ticks from some unspecified point in the past monotonically. You can't compare its value from different machines. The only guarantee is that it never goes backward and increments monotonically.
BTW, we can use something like NTP to get an accurate value of monotonic clock on remote machine. So we can calculate the offset between local monotonic clock and some cluster coordinated time (initially obtained from cluster leader or something like that), and then use it for synchronization between machines. This way we can guarantee that time is actually synchronized.

I've actually never used GetTickCount64 in a .NET application

People usually use Timer, Task.Delay, Stopwatch or just don't care about NTP, daylight saving time, etc.

[Aaronontheweb]

BTW, we can use something like NTP to get an accurate value of monotonic clock on remote machine.

For our purposes, I don't think this is necessary - all of the scheduling in Akka.NET is done locally. Just needs to happen on regular intervals, not at specific dates and times.

As for time-ordered network events, our built-in ones use vector clocks that don't take any dependencies on the CLR time system:

https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka.Cluster/VectorClock.cs

[nvivo]

A little late for the discussion... I completely agree on moving from DateTime to a monotonic clock.

But does akka.net spend so much time requesting the current time that it requires worrying about the performance of QueryPerformanceCounter? I can call this method 8 million times/second on my laptop on debug mode attached to visual studio with energy savings on. This goes to 20 million/second on release mode...

From what I measured, it's ~4 times slower than DateTime.UtcNow, and ~8 times slower than Environment.TickCount. But should it matter?

All I'm saying is that maybe we should first see what is really required. Does Akka.net need nanosecond precision? Then Stopwatch is the best answer. If it's used for scheduling, maybe Environment.TickCount works. Does it need both? So expose both.

For reference, TPL uses Timers, which use Environment.TickCount until Windows 7, that means you can't schedule tasks for more than 29 days anyway. If it works for TPL, should work for Akka.

Apparently the Mono people agree.

[Aaronontheweb]

@nvivo the performance is less of a concern than DST / users tampering with the clock IMHO.

For reference, TPL uses Timers, which use Environment.TickCount until Windows 7, that means you can't schedule tasks for more than 29 days anyway. If it works for TPL, should work for Akka.

The design concerns that the TPL and Akka.NET have overlap by only a very small amount. Discuss designs on their own merits; not by appealing to the authority of another library. Stick to what's important for Akka.

In this case - raises a question I hadn't thought about: what mechanism is most effective for ensuring consistent scheduling over very long periods of time? Wrap TickCount in an unchecked block?

Users should be able to have long-running actors that run for months without restart - and they should be able to use a scheduler that entire time.

[Aaronontheweb]

Looks like the answer to my question lies in the comments on https://msdn.microsoft.com/en-us/library/windows/desktop/ms724411%28v=vs.85%29.aspx

If GetTickCount is 49.7 days, how long is GetTickCount64?
It's ~585 million years.

Tergiver
2/14/2011
GetTickCount64 is in milliseconds, not ticks
Despite the name, it returns the number of milliseconds of uptime, not ticks.

Sounds like @kekekeks's PR should work for that scenario then.

[nvivo]

Just a side note, the int32 part shouldn't affect anything in a system running for more than 29 or 47 days.

But that's a good solution anyway.

[rogeralsing]

Can this be closed? we did merge the Monotonic clock PR

[kekekeks]

Motonic clock is yet to be integrated into the code base.

[kekekeks]

I think we can close this.