golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.21k stars 17.7k forks source link

time: Round does not work with 7ms. #70449

Closed KyleSanderson closed 2 days ago

KyleSanderson commented 2 days ago

Go version

go1.23.2 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Carlos\AppData\Local\go-build
set GOENV=C:\Users\Carlos\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Carlos\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Carlos\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.23.2
set GODEBUG=
set GOTELEMETRY=local
set GOTELEMETRYDIR=C:\Users\Carlos\AppData\Roaming\go\telemetry
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\Carlos\AppData\Local\Temp\go-build825637060=/tmp/go-build -gno-record-gcc-switches

What did you do?

https://go.dev/play/p/Mvbh3SQwa3b

// You can edit this code!
// Click here and start typing.
package main

import (
    "fmt"
    "time"
)

func main() {
    const rounds = 700

    for k := 1; k < 37; k++ {
        unique := 0
        old := time.Now().Round(time.Millisecond * time.Duration(k)).UnixMilli()
        for i := 0; i < rounds; i++ {
            new := time.Now().Round(time.Millisecond * time.Duration(k)).UnixMilli()
            if new > old {
                unique++
                old = new
            }

            if div := new % int64(k); div != 0 {
                fmt.Printf("%d is broken. not a multiple of %d\n", k, div)
                break
            }
            time.Sleep(time.Millisecond * 1)
        }

        if unique < rounds/k-1 {
            fmt.Printf("%d is broken. failed round count %d\n", k, unique)
            continue
        }
    }
}

What did you see happen?

7 is broken. not a multiple of 4 7 is broken. failed round count 0 11 is broken. not a multiple of 2 11 is broken. failed round count 0 13 is broken. not a multiple of 4 13 is broken. failed round count 0 14 is broken. not a multiple of 4 14 is broken. failed round count 0 17 is broken. not a multiple of 11 17 is broken. failed round count 0 19 is broken. not a multiple of 18 19 is broken. failed round count 0 21 is broken. not a multiple of 18 21 is broken. failed round count 0 22 is broken. not a multiple of 2 22 is broken. failed round count 0 23 is broken. not a multiple of 11 23 is broken. failed round count 0 26 is broken. not a multiple of 4 26 is broken. failed round count 0 28 is broken. not a multiple of 4 28 is broken. failed round count 0 29 is broken. not a multiple of 3 29 is broken. failed round count 0 31 is broken. not a multiple of 13 31 is broken. failed round count 0 33 is broken. not a multiple of 24 33 is broken. failed round count 0 34 is broken. not a multiple of 28 34 is broken. failed round count 0 35 is broken. not a multiple of 25 35 is broken. failed round count 0

What did you expect to see?

.

Something's not right with the rounding code. 5 and 10 work, but once you start to get up there things fall apart fast.

gabyhelp commented 2 days ago

Related Issues

Related Code Changes

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

seankhliao commented 2 days ago

see https://pkg.go.dev/time#Time.Round

Round operates on the time as an absolute duration since the zero time; it does not operate on the presentation form of the time. Thus, Round(Hour) may return a time with a non-zero minute, depending on the time's Location.

Zero time is not unix 0, it's offset by 62135596800000 milliseconds, so division based in unix milliseconds is wrong.

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.

For questions please refer to https://github.com/golang/go/wiki/Questions

KyleSanderson commented 2 days ago

@seankhliao I'm struggling to see how this is not a bug, even with a local tz offset this appears to be a very clearly not intended. If you're confident this isn't a bug I'm happy to jump to Questions, but the smallest unit here is Nanoseconds, and Round still does not work properly with the same defective integers.

This one liner also reliably reproduces this at the nanosecond scale: https://go.dev/play/p/bNALNfokU69

package main

import (
    "fmt"
    "time"
)

func main() {
    fmt.Printf("%d", time.Now().Round(time.Nanosecond*7).Nanosecond())
}
3

https://go.dev/play/p/OwyyQIRhWWl

package main

import (
    "fmt"
    "time"
)

func main() {
    const rounds = 700

    for k := 1; k < 37; k++ {
        unique := 0
        old := time.Now().Round(time.Nanosecond * time.Duration(k))
        for i := 0; i < rounds; i++ {
            new := time.Now().Round(time.Nanosecond * time.Duration(k))
            if new.After(old) {
                unique++
                old = new
            }

            if div := new.Nanosecond() % k; div != 0 {
                fmt.Printf("%d is broken. not a multiple of %d\n", k, div)
                break
            }
            time.Sleep(time.Nanosecond * 1)
        }

        if unique < rounds/k-1 {
            fmt.Printf("%d is broken. failed round of %d\n", k, unique)
            continue
        }
    }
}
7 is broken. not a multiple of 3
7 is broken. failed round of 0
11 is broken. not a multiple of 2
11 is broken. failed round of 0
13 is broken. not a multiple of 3
13 is broken. failed round of 0
14 is broken. not a multiple of 10
14 is broken. failed round of 0
17 is broken. not a multiple of 15
17 is broken. failed round of 0
19 is broken. not a multiple of 15
19 is broken. failed round of 0
21 is broken. not a multiple of 3
21 is broken. failed round of 0
22 is broken. not a multiple of 2
22 is broken. failed round of 0
23 is broken. not a multiple of 7
23 is broken. failed round of 0
26 is broken. not a multiple of 16
26 is broken. failed round of 0
27 is broken. not a multiple of 9
27 is broken. failed round of 0
28 is broken. not a multiple of 24
28 is broken. failed round of 0
29 is broken. not a multiple of 5
29 is broken. failed round of 0
31 is broken. not a multiple of 29
31 is broken. failed round of 0
33 is broken. not a multiple of 24
33 is broken. failed round of 0
34 is broken. not a multiple of 32
34 is broken. failed round of 0
35 is broken. not a multiple of 10
35 is broken. failed round of 0
seankhliao commented 2 days ago

It is not a bug.

package main

import (
    "fmt"
    "time"
)

func main() {
    t0 := time.Time{}
    t1 := time.Now().Round(7 * time.Millisecond)
    unixMilli := t1.UnixMilli()
    absMilli := (-t0.UnixMilli()) + unixMilli
    fmt.Println(unixMilli % 7) // 4
    fmt.Println(absMilli % 7) // 0
}
KyleSanderson commented 2 days ago

This just seems wild. (Now I'm trying to figure out) How do I properly call time.Now() so that Round or Truncate reliably work?

https://go.dev/play/p/KxqhNtxQKoH

package main

import (
    "fmt"
    "time"
)

func main() {
    t := time.Now()
    for i := 0; i < 99; i++ {
        fmt.Printf("%v = %v\n", i, t.Truncate(time.Nanosecond*time.Duration(i)).Nanosecond())
    }
}
0 = 0
1 = 0
2 = 0
3 = 0
4 = 0
5 = 0
6 = 0
7 = 999999996
8 = 0
9 = 0
10 = 0
11 = 999999991
12 = 0
13 = 999999990
14 = 999999996
15 = 0
16 = 0
17 = 999999998
18 = 0
19 = 999999996
20 = 0
21 = 999999982
22 = 999999980
23 = 999999984
24 = 0
25 = 0
26 = 999999990
27 = 999999982
28 = 999999996
29 = 999999976
30 = 0
31 = 999999998
32 = 0
33 = 999999991
34 = 999999998
35 = 999999975
36 = 0
37 = 999999993
38 = 999999996
39 = 999999964
40 = 0
41 = 999999990
42 = 999999982
43 = 999999992
44 = 999999980
45 = 0
46 = 999999984
47 = 999999960
48 = 0
49 = 999999968
50 = 0
51 = 999999964
52 = 999999964
53 = 0
54 = 999999982
55 = 999999980
56 = 999999968
57 = 999999958
58 = 999999976
59 = 999999976
60 = 0
61 = 999999955
62 = 999999998
63 = 999999982
64 = 0
65 = 999999990
66 = 999999958
67 = 999999980
68 = 999999964
69 = 999999961
70 = 999999940
71 = 999999942
72 = 0
73 = 999999955
74 = 999999956
75 = 0
76 = 999999996
77 = 999999947
78 = 999999964
79 = 999999968
80 = 0
81 = 999999928
82 = 999999990
83 = 999999935
84 = 999999940
85 = 999999930
86 = 999999992
87 = 999999976
88 = 999999936
89 = 999999963
90 = 0
91 = 999999912
92 = 999999984
93 = 999999967
94 = 999999960
95 = 999999920
96 = 0
97 = 999999989
98 = 999999968
seankhliao commented 2 days ago

Please take your questions to the forums, Round is working as documented.

KyleSanderson commented 2 days ago

https://github.com/golang/go/issues/16647