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

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:
10 years, 10 months ago by jayschwa
Modified:
10 years, 8 months ago
Reviewers:
gobot, rsc, dvyukov
CC:
golang-codereviews, dvyukov, brainman, slimsag, dave_cheney.net, 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/
10 years, 10 months 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 ...
10 years, 10 months 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 ...
10 years, 10 months ago (2014-01-16 15:03:35 UTC) #3
jayschwa
Jan 18 update: Implemented monotonic clock for Windows.
10 years, 10 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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: > > > > ...
10 years, 9 months 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 ...
10 years, 9 months ago (2014-02-05 05:54:30 UTC) #18
dave_cheney.net
Whoa, TSC alert. The TSC cannot be reliably used from user space without elaborate precautions. ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months 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 ...
10 years, 9 months ago (2014-02-05 16:37:15 UTC) #22
rsc
Please revert the windows changes and then we can submit this CL as is.
10 years, 9 months 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 ...
10 years, 9 months ago (2014-02-13 02:37:16 UTC) #24
dvyukov
LGTM again
10 years, 9 months 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 ...
10 years, 8 months ago (2014-02-21 20:20:18 UTC) #26
rsc
LGTM
10 years, 8 months 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 ...
10 years, 8 months 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 ...
10 years, 8 months 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` ...
10 years, 8 months 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, ...
10 years, 8 months 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 ...
10 years, 8 months 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 ...
10 years, 8 months ago (2014-02-24 15:57:50 UTC) #33
gobot
10 years, 8 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