Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(330)

Issue 53010043: code review 53010043: runtime: Use same [monotonic] reference clock for all timer values (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by jayschwa
Modified:
4 years, 11 months ago
Reviewers:
gobot, rsc, dvyukov
CC:
golang-codereviews, dvyukov, brainman, slimsag, dfc, rsc, mikio
Visibility:
Public.

Description

runtime: Use same [monotonic] reference clock for all timer values This lays the groundwork for making Go robust when the system's calendar time jumps around. All input values to the runtimeTimer struct now use the runtime clock as a common reference point. This affects net.Conn.Set[Read|Write]Deadline(), time.Sleep(), time.Timer, etc. Under normal conditions, behavior is unchanged. Each platform and architecture's implementation of runtime·nanotime() should be modified to use a monotonic system clock when possible. Platforms/architectures modified and tested with monotonic clock: linux/x86 - clock_gettime(CLOCK_MONOTONIC) Update issue 6007

Patch Set 1 #

Patch Set 2 : diff -r 761cf9b4e530 https://code.google.com/p/go/ #

Patch Set 3 : diff -r 761cf9b4e530 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 322f646feecb https://code.google.com/p/go #

Patch Set 5 : diff -r 322f646feecb https://code.google.com/p/go #

Total comments: 1

Patch Set 6 : diff -r 5b5a2d2d2563 https://code.google.com/p/go #

Total comments: 6

Patch Set 7 : diff -r 2df447356d7f https://code.google.com/p/go #

Total comments: 2

Patch Set 8 : diff -r da649606c628 https://code.google.com/p/go #

Patch Set 9 : diff -r e4a4cb47c141 https://code.google.com/p/go #

Patch Set 10 : diff -r 51d9b0cf3b8d https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -13 lines) Patch
M src/pkg/net/fd_poll_runtime.go View 1 2 chunks +4 lines, -1 line 0 comments Download
M src/pkg/runtime/netpoll.goc View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -0 lines 0 comments Download
M src/pkg/runtime/sys_linux_386.s View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/runtime/sys_linux_amd64.s View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/runtime/time.goc View 1 1 chunk +5 lines, -0 lines 0 comments Download
M src/pkg/time/internal_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/time/sleep.go View 1 3 chunks +5 lines, -7 lines 0 comments Download
M src/pkg/time/tick.go View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 34
jayschwa
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
5 years, 1 month ago (2014-01-16 06:35:56 UTC) #1
dvyukov
LGTM When/if it is submitted, please also file an issue to support monotonic time on ...
5 years, 1 month ago (2014-01-16 07:23:34 UTC) #2
jayschwa
On 2014/01/16 07:23:34, dvyukov wrote: > LGTM > > When/if it is submitted, please also ...
5 years, 1 month ago (2014-01-16 15:03:35 UTC) #3
jayschwa
Jan 18 update: Implemented monotonic clock for Windows.
5 years, 1 month ago (2014-01-18 09:02:27 UTC) #4
brainman
This fails on windows: C:\go\root\src>all.bat # Building C bootstrap tool. cmd/dist # Building compilers and ...
5 years, 1 month ago (2014-01-20 01:35:09 UTC) #5
jayschwa
I'm unable to reproduce the fatal stack split on my machine. 386 and amd64 are ...
5 years, 1 month ago (2014-01-20 02:07:07 UTC) #6
brainman
On 2014/01/20 02:07:07, jayschwa wrote: > I'm unable to reproduce the fatal stack split on ...
5 years, 1 month ago (2014-01-20 02:30:56 UTC) #7
jayschwa(gmail)
I uploaded a change to check the return of QueryPerformanceFrequency. I am not sure how ...
5 years, 1 month ago (2014-01-20 03:33:11 UTC) #8
jayschwa(gmail)
I uploaded a change to check the return of QueryPerformanceFrequency. I am not sure how ...
5 years, 1 month ago (2014-01-20 03:33:11 UTC) #9
brainman
On 2014/01/20 03:33:11, jayschwa(gmail) wrote: > ... > I am not sure how to go ...
5 years, 1 month ago (2014-01-20 06:34:08 UTC) #10
dvyukov
https://codereview.appspot.com/53010043/diff/80001/src/pkg/runtime/os_windows.c File src/pkg/runtime/os_windows.c (right): https://codereview.appspot.com/53010043/diff/80001/src/pkg/runtime/os_windows.c#newcode272 src/pkg/runtime/os_windows.c:272: nsec_per_tick = 1000000000LL / ticks; on 32 bits 64-bit ...
5 years ago (2014-01-25 11:27:06 UTC) #11
jayschwa
dvyukov, I have replaced 64-bit division with timediv(). Thanks for the tip. brainman, can you ...
5 years ago (2014-01-25 23:59:34 UTC) #12
brainman
On 2014/01/25 23:59:34, jayschwa wrote: > ... > brainman, can you retry the latest update ...
5 years ago (2014-01-29 01:57:21 UTC) #13
jayschwa
Thanks for the report. I've seen that sort of error when nanotime() or now() (or ...
5 years ago (2014-01-29 03:08:58 UTC) #14
jayschwa
Hmm, well I fired up a 32-bit Windows Server 2008 instance on EC2 and can't ...
5 years ago (2014-01-29 04:48:42 UTC) #15
brainman
On 2014/01/29 04:48:42, jayschwa wrote: > > Can you provide some more details about your ...
5 years ago (2014-01-29 06:02:59 UTC) #16
slimsag
On 2014/01/29 06:02:59, brainman wrote: > On 2014/01/29 04:48:42, jayschwa wrote: > > > > ...
5 years ago (2014-02-05 05:29:11 UTC) #17
brainman
On 2014/02/05 05:29:11, stephen.gutekanst wrote: > > ... Brainman, perhaps you have hardware that is ...
5 years ago (2014-02-05 05:54:30 UTC) #18
dfc
Whoa, TSC alert. The TSC cannot be reliably used from user space without elaborate precautions. ...
5 years ago (2014-02-05 05:57:13 UTC) #19
dvyukov
Try this one: http://play.golang.org/p/200I8kC3Da It is not subject to jumps and gives you 1ms precision ...
5 years ago (2014-02-05 06:16:57 UTC) #20
brainman
On 2014/02/05 06:16:57, dvyukov wrote: > ... I was using *(unsigned*)0x7FFE0000 to read > current ...
5 years ago (2014-02-05 06:26:13 UTC) #21
jayschwa
On 2014/02/05 05:29:11, stephen.gutekanst wrote: > How does this CL intend to handle issues regarding ...
5 years ago (2014-02-05 16:37:15 UTC) #22
rsc
Please revert the windows changes and then we can submit this CL as is.
5 years ago (2014-02-12 18:05:55 UTC) #23
jayschwa
On 2014/02/12 18:05:55, rsc wrote: > Please revert the windows changes and then we can ...
5 years ago (2014-02-13 02:37:16 UTC) #24
dvyukov
LGTM again
5 years ago (2014-02-13 06:12:25 UTC) #25
jayschwa
go-dev.appspot.com says the status of this review is "waiting for author". Is there some additional ...
5 years ago (2014-02-21 20:20:18 UTC) #26
rsc
LGTM
5 years ago (2014-02-21 20:27:57 UTC) #27
rsc
I tried to submit this but it fails on my Mac. At first glance I ...
5 years ago (2014-02-21 20:31:53 UTC) #28
jayschwa
I'm unable to reproduce. A fresh clone, clpatch, and all.bash yields ALL TESTS PASSED on ...
5 years ago (2014-02-21 20:51:37 UTC) #29
jayschwa
After further testing, I can intermittently reproduce the errors by manually running `go test net` ...
5 years ago (2014-02-21 22:52:41 UTC) #30
mikio
Please make sure which test case fails. If that's TestVariousDeadlines1Proc or TestVariousDeadlines4Proc, no worries. Otherwise, ...
5 years ago (2014-02-22 01:20:33 UTC) #31
jayschwa
On 2014/02/22 01:20:33, mikio wrote: > TestVariousDeadlines1Proc or TestVariousDeadlines4Proc Those are indeed the ones I ...
5 years ago (2014-02-22 01:39:38 UTC) #32
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=79f855ac890d *** runtime: use monotonic clock for timers (linux/386, linux/amd64) This lays ...
4 years, 12 months ago (2014-02-24 15:57:50 UTC) #33
gobot
4 years, 12 months ago (2014-02-25 00:33:13 UTC) #34
This CL appears to have broken the openbsd-386-rootbsd builder.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b