time: optimize time.Time.Sub and time.Since #17858
Comments
dsnet
changed the title
|
|
dsnet
changed the title
|
Oddly, looking at the output of |
|
CL https://golang.org/cl/32971 mentions this issue. |
|
// Add returns the time t+d.
func (t Time) Add(d Duration) Time {
t.sec += int64(d / 1e9)
t.nsec += int32(d % 1e9)
if t.nsec >= 1e9 {
t.sec++
t.nsec -= 1e9
} else if t.nsec < 0 {
t.sec--
t.nsec += 1e9
}
return t
}
// Sub returns the duration t-u. If the result exceeds the maximum (or minimum)
// value that can be stored in a Duration, the maximum (or minimum) duration
// will be returned.
// To compute t-d for a duration d, use t.Add(-d).
func (t Time) Sub(u Time) Duration {
d := Duration(t.sec-u.sec)*Second + Duration(t.nsec-u.nsec)
// Check for overflow or underflow.
switch {
case u.Add(d).Equal(t):
return d // d is correct
case t.Before(u):
return minDuration // t - u is negative out of range
}
return maxDuration // t - u is positive out of range
} |
|
I looked at this earlier today, and I am suspicious of the current overflow checking logic. This is a version of func (t Time) SubC(u Time) Duration {
sec := t.sec - u.sec
nsec := t.nsec - u.nsec // Cannot overflow; in range of [-999999999, 999999999]
d1 := Duration(sec) * Second
d2 := d1 + Duration(nsec) // Overflows only when nsec < 0
overflow := (sec < u.sec) != (t.sec > 0) || // Subtraction overflow?
(d1/Second) != Duration(sec) || // Multiplication overflow?
(d2 < d1) != (nsec < 0) // Addition overflow?
if !overflow {
return d2
}
// Overflow can only occur if number of seconds apart is large enough.
// Thus, we do not need to compare the nanoseconds.
if t.sec < u.sec {
return minDuration
} else {
return maxDuration
}
}On my machine, this version runs 3.2x faster. |
Many non-inlineable functions were not being reported in '-m -m' mode. Updates #17858. Change-Id: I7d96361b39dd317f5550e57334a8a6dd1a836598 Reviewed-on: https://go-review.googlesource.com/32971 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
Sadly bumping this to Go 1.10. |
|
The inclusion of monotonic timestamps doubled the complexity of the |
|
If this is ever made inlineable, be sure to add it to |
|
Change https://golang.org/cl/107056 mentions this issue: |
Existing tests don't check overflow and underflow case for subtraction monotonic time. Updates #17858 Change-Id: I95311440134c92eadd7d5e409a0fc7c689e9bf41 Reviewed-on: https://go-review.googlesource.com/107056 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
Change https://golang.org/cl/131196 mentions this issue: |
This is primarily achieved by checking for arithmetic overflow instead of using Add and Equal. It's a decent performance improvement even though the function still isn't inlined. name old time/op new time/op delta Sub-6 242ns ± 0% 122ns ± 0% -49.59% (p=0.002 n=8+10) Updates #17858. Change-Id: I1469b618183c83ea8ea54d5ce277eb15f2ec0f11 Reviewed-on: https://go-review.googlesource.com/c/go/+/131196 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
https://golang.org/cl/131196 was reverted in https://golang.org/cl/190497 because it's time calculation was incorrect in some edge cases. |
|
Change https://golang.org/cl/190524 mentions this issue: |
CL 131196 optimized Time.Sub, but was reverted because it incorrectly computed the nanoseconds in some edge cases. This CL adds a test case to enforce the correct behavior so that a future optimization does not break this again. Updates #17858 Updates #33677 Change-Id: I596d8302ca6bf721cf7ca11cc6f939639fcbdd43 Reviewed-on: https://go-review.googlesource.com/c/go/+/190524 Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Andrew Bonventre <andybons@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 131196 optimized Time.Sub, but was reverted because it incorrectly computed the nanoseconds in some edge cases. This CL adds a test case to enforce the correct behavior so that a future optimization does not break this again. Updates golang#17858 Updates golang#33677 Change-Id: I596d8302ca6bf721cf7ca11cc6f939639fcbdd43 Reviewed-on: https://go-review.googlesource.com/c/go/+/190524 Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Andrew Bonventre <andybons@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
According to a profiling data from a large number of servers at Google,
time.Time.Subis within the top 150 functions by CPU time. The current implementation ofSubis not inlineable. The logic forSubis not particularly complicated and I believe it can be made to be inlined with some love.The text was updated successfully, but these errors were encountered: