Issues with time.Time
503 views
Skip to first unread message
Andrew Phillips
Jul 17, 2022, 9:20:17 AM7/17/22
to golang-nuts
I've discovered a few problems with time.Time but thought I better discuss here first before creating issues at https://github.com/golang/go/issues (in case I missed something obvious :). These are mainly performance issues, but there is also the serious problem that it is easy to accidentally disable "monotonic" time differences (reverting to "wall" time) and that you cannot obtain a monotonic time with *Location == nil (UTC).
This all began with real software that generates (using time.Now()) and retains millions of timestamps (in memory) used for various purposes including the deletions of old data by comparing these times with "now". Due to the large number of calls to time.Now() performance is important, accounting for a significant proportion of processing time.
The first optimization made (some time ago) was to replace calls to time.Now() with time.Now.UTC() in an attempt to reduce the load on the garbage collector. (Calling the UTC() method sets the *Location field to nil which obviates the GC need to follow the pointer I believe.)
Recent benchmarking revealed that calling calling time.Now().UTC() takes more than twice as long as calling time.Now(). Moreover, time.Now() has some code that is effectively "dead" (never executed) which could reduce that it by another 30% -- there is an if statement that will always return false until after the current time is well into the future (after 2150 or so).
A further issue I found from inspecting the code for the UTC() method is that calling it clears the hasMonotonic flag and subsequent time comparisons use "wall" time - this affects many calls to ContextWithDeadline(). The only discussion I can find on why calling UTC() should clear the hasMonotonic flag is this comment from Russ Cox [https://github.com/golang/go/issues/18991#issuecomment-306209288] which does not explain what tests are fixed or even what is being tested.
I find it particularly worrisome that you can easily and inadvertently turn off "monotonic" time comparisons.
Some changes I might propose are:
1. Removal of the dead code in time.Now()
2. Calling UTC() should not clear the hasMonotonic flag
3. Passing a non-monotonic time to contextWithDeadline should panic
4. Add a new function (eg, time.NowUTC()) to efficiently get the current monotonic, UTC time
These changes (except for 4) have backward compatibility implications which I have considered (and believe will be acceptable) but will need further research and discussion.
This all began with real software that generates (using time.Now()) and retains millions of timestamps (in memory) used for various purposes including the deletions of old data by comparing these times with "now". Due to the large number of calls to time.Now() performance is important, accounting for a significant proportion of processing time.
The first optimization made (some time ago) was to replace calls to time.Now() with time.Now.UTC() in an attempt to reduce the load on the garbage collector. (Calling the UTC() method sets the *Location field to nil which obviates the GC need to follow the pointer I believe.)
Recent benchmarking revealed that calling calling time.Now().UTC() takes more than twice as long as calling time.Now(). Moreover, time.Now() has some code that is effectively "dead" (never executed) which could reduce that it by another 30% -- there is an if statement that will always return false until after the current time is well into the future (after 2150 or so).
A further issue I found from inspecting the code for the UTC() method is that calling it clears the hasMonotonic flag and subsequent time comparisons use "wall" time - this affects many calls to ContextWithDeadline(). The only discussion I can find on why calling UTC() should clear the hasMonotonic flag is this comment from Russ Cox [https://github.com/golang/go/issues/18991#issuecomment-306209288] which does not explain what tests are fixed or even what is being tested.
I find it particularly worrisome that you can easily and inadvertently turn off "monotonic" time comparisons.
Some changes I might propose are:
1. Removal of the dead code in time.Now()
2. Calling UTC() should not clear the hasMonotonic flag
3. Passing a non-monotonic time to contextWithDeadline should panic
4. Add a new function (eg, time.NowUTC()) to efficiently get the current monotonic, UTC time
These changes (except for 4) have backward compatibility implications which I have considered (and believe will be acceptable) but will need further research and discussion.
Ian Lance Taylor
Jul 18, 2022, 3:34:19 PM7/18/22
to Andrew Phillips, golang-nuts
On Sun, Jul 17, 2022 at 6:20 AM Andrew Phillips <aphill...@gmail.com> wrote:
>
> I've discovered a few problems with time.Time but thought I better discuss here first before creating issues at https://github.com/golang/go/issues (in case I missed something obvious :). These are mainly performance issues, but there is also the serious problem that it is easy to accidentally disable "monotonic" time differences (reverting to "wall" time) and that you cannot obtain a monotonic time with *Location == nil (UTC).
>
> This all began with real software that generates (using time.Now()) and retains millions of timestamps (in memory) used for various purposes including the deletions of old data by comparing these times with "now". Due to the large number of calls to time.Now() performance is important, accounting for a significant proportion of processing time.
>
> The first optimization made (some time ago) was to replace calls to time.Now() with time.Now.UTC() in an attempt to reduce the load on the garbage collector. (Calling the UTC() method sets the *Location field to nil which obviates the GC need to follow the pointer I believe.)
I wouldn't expect that change to make a big difference, but it should
>
> I've discovered a few problems with time.Time but thought I better discuss here first before creating issues at https://github.com/golang/go/issues (in case I missed something obvious :). These are mainly performance issues, but there is also the serious problem that it is easy to accidentally disable "monotonic" time differences (reverting to "wall" time) and that you cannot obtain a monotonic time with *Location == nil (UTC).
>
> This all began with real software that generates (using time.Now()) and retains millions of timestamps (in memory) used for various purposes including the deletions of old data by comparing these times with "now". Due to the large number of calls to time.Now() performance is important, accounting for a significant proportion of processing time.
>
> The first optimization made (some time ago) was to replace calls to time.Now() with time.Now.UTC() in an attempt to reduce the load on the garbage collector. (Calling the UTC() method sets the *Location field to nil which obviates the GC need to follow the pointer I believe.)
help a little. What would help more is converting your stored
time.Time values by calling the UnixNano method, as that would give
you a value with no pointers at all. However, that would of course
lose the monotonic time reading.
> Recent benchmarking revealed that calling calling time.Now().UTC() takes more than twice as long as calling time.Now(). Moreover, time.Now() has some code that is effectively "dead" (never executed) which could reduce that it by another 30% -- there is an if statement that will always return false until after the current time is well into the future (after 2150 or so).
benchmark code? Note that this is an area where microbenchmarks can
be fairly misleading. Real code does not call time.Now in a tight
loop without other memory writes.
> A further issue I found from inspecting the code for the UTC() method is that calling it clears the hasMonotonic flag and subsequent time comparisons use "wall" time - this affects many calls to ContextWithDeadline(). The only discussion I can find on why calling UTC() should clear the hasMonotonic flag is this comment from Russ Cox [https://github.com/golang/go/issues/18991#issuecomment-306209288] which does not explain what tests are fixed or even what is being tested.
https://pkg.go.dev/time, which says "Because t.In, t.Local, and t.UTC
are used for their effect on the interpretation of the wall time, they
also strip any monotonic clock reading from their results."
> I find it particularly worrisome that you can easily and inadvertently turn off "monotonic" time comparisons.
>
> Some changes I might propose are:
> 1. Removal of the dead code in time.Now()
instructions including a jump that should always be predicted
correctly. We would need to see some very clear evidence that those
four instructions weigh heavily compared to the couple of hundred
instructions required to fetch the current time value (on
linux-amd64).
> 2. Calling UTC() should not clear the hasMonotonic flag
> 3. Passing a non-monotonic time to contextWithDeadline should panic
Programs construct deadlines in all sorts of ways.
> 4. Add a new function (eg, time.NowUTC()) to efficiently get the current monotonic, UTC time
makes a difference for multiple real programs. My guess, without any
data, would be that in the vast majority of programs time.Time values
are not a significant percentage of memory. And the default value of
the pointer returned by time.Now will be a pointer to the data section
rather than the heap, so the GC should discard the pointer with a few
comparisons. So it's not obvious why it would be a significant source
of GC time.
Ian
Slawomir Pryczek
Jul 19, 2022, 11:20:48 AM7/19/22
to golang-nuts
Have you tried starting a simple thread which will update some variable every 100ms (for example), and then just get value of this variable? Using atomic functions. I believe memcached was 'caching' calls to get time this way and it won't probably be very accurate (as we don't have hard quarantees for doing sleep(..)) but even if it'd be +2 seconds off that shouldn't make an issue.
Andrew Phillips
Jul 21, 2022, 10:37:16 PM7/21/22
to golang-nuts
Thanks for the feedback. I should explain that efficiency is not (currently) a problem.
The thing I love about Go is that there is always a simple and obvious (and reasonably efficient) way to do things. I'm currently call time.Now().UTC() no more than a few thousand times per second and store not much more 1G (1e9) time.Times in RAM at once. I have considered things like "caching" the current (monotonic) time (only calling time.Now once per millisecond but incrementing to ensure each time is unique) and storing an 8-byte duration from a fixed time instead of a 24-bit time.Time (saving many GB of RAM). However, this will complicate the code and we currently have more than enough CPU and RAM.
My only current problem is that I have been for years creating deadlines using Times with hasMonotonic turned off, while I unknowingly assured my boss that I was using a "steady" clock. It never occurred to me that this would be a side-effect of calling the UTC() method.
The thing I love about Go is that there is always a simple and obvious (and reasonably efficient) way to do things. I'm currently call time.Now().UTC() no more than a few thousand times per second and store not much more 1G (1e9) time.Times in RAM at once. I have considered things like "caching" the current (monotonic) time (only calling time.Now once per millisecond but incrementing to ensure each time is unique) and storing an 8-byte duration from a fixed time instead of a 24-bit time.Time (saving many GB of RAM). However, this will complicate the code and we currently have more than enough CPU and RAM.
My only current problem is that I have been for years creating deadlines using Times with hasMonotonic turned off, while I unknowingly assured my boss that I was using a "steady" clock. It never occurred to me that this would be a side-effect of calling the UTC() method.
Reply all
Reply to author
Forward
0 new messages