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

Issue 6820120: code review 6820120: runtime: use clock_gettime to get ns resolution for tim... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 5 months ago by minux1
Modified:
12 years, 4 months ago
Reviewers:
CC:
jsing, dave_cheney.net, rsc, golang-dev
Visibility:
Public.

Description

runtime: use clock_gettime to get ns resolution for time.now & runtime.nanotime For Linux/{386,arm}, FreeBSD/{386,amd64,arm}, NetBSD/{386,amd64}, OpenBSD/{386,amd64}. Note: our Darwin implementation already has ns resolution. Linux/386 (Core i7-2600 @ 3.40GHz, kernel 3.5.2-gentoo) benchmark old ns/op new ns/op delta BenchmarkNow 110 118 +7.27% Linux/ARM (ARM Cortex-A8 @ 800MHz, kernel 2.6.32.28 android) benchmark old ns/op new ns/op delta BenchmarkNow 625 542 -13.28% Linux/ARM (ARM Cortex-A9 @ 1GHz, Pandaboard) benchmark old ns/op new ns/op delta BenchmarkNow 992 909 -8.37% FreeBSD 9-REL-p1/amd64 (Dell R610 Server with Xeon X5650 @ 2.67GHz) benchmark old ns/op new ns/op delta BenchmarkNow 699 695 -0.57% FreeBSD 9-REL-p1/amd64 (Atom D525 @ 1.80GHz) benchmark old ns/op new ns/op delta BenchmarkNow 1553 1658 +6.76% OpenBSD/amd64 (Dell E6410 with i5 CPU M 540 @ 2.53GHz) benchmark old ns/op new ns/op delta BenchmarkNow 1262 1236 -2.06% OpenBSD/i386 (Asus eeePC 701 with Intel Celeron M 900MHz - locked to 631MHz) benchmark old ns/op new ns/op delta BenchmarkNow 5089 5043 -0.90% NetBSD/i386 (VMware VM with Core i5 CPU @ 2.7GHz) benchmark old ns/op new ns/op delta BenchmarkNow 277 278 +0.36% NetBSD/amd64 (VMware VM with Core i5 CPU @ 2.7Ghz) benchmark old ns/op new ns/op delta BenchmarkNow 103 105 +1.94% Thanks Maxim Khitrov, Joel Sing, and Dave Cheney for providing benchmark data.

Patch Set 1 #

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

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

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

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

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

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

Total comments: 4

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

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

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

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

Patch Set 12 : diff -r b7482cb67996 https://code.google.com/p/go/ #

Total comments: 3

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

Total comments: 2

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

Patch Set 15 : diff -r 3684de5292bf https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -119 lines) Patch
M doc/go1.1.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +4 lines, -4 lines 0 comments Download
M src/pkg/runtime/sys_freebsd_386.s View 1 2 3 4 5 6 7 8 9 10 1 chunk +11 lines, -13 lines 0 comments Download
M src/pkg/runtime/sys_freebsd_amd64.s View 1 2 3 4 5 6 1 chunk +10 lines, -12 lines 0 comments Download
M src/pkg/runtime/sys_freebsd_arm.s View 1 2 3 1 chunk +12 lines, -15 lines 0 comments Download
M src/pkg/runtime/sys_linux_386.s View 1 2 3 4 5 6 1 chunk +11 lines, -13 lines 0 comments Download
M src/pkg/runtime/sys_linux_arm.s View 1 2 3 4 2 chunks +10 lines, -13 lines 0 comments Download
M src/pkg/runtime/sys_netbsd_386.s View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +8 lines, -10 lines 0 comments Download
M src/pkg/runtime/sys_netbsd_amd64.s View 1 2 3 4 5 6 7 8 9 10 1 chunk +10 lines, -12 lines 0 comments Download
M src/pkg/runtime/sys_openbsd_386.s View 1 2 3 4 5 6 7 8 9 10 1 chunk +11 lines, -13 lines 0 comments Download
M src/pkg/runtime/sys_openbsd_amd64.s View 1 2 3 4 5 6 7 8 9 10 1 chunk +12 lines, -14 lines 0 comments Download

Messages

Total messages: 13
jsing
https://codereview.appspot.com/6820120/diff/2009/src/pkg/runtime/sys_openbsd_amd64.s File src/pkg/runtime/sys_openbsd_amd64.s (right): https://codereview.appspot.com/6820120/diff/2009/src/pkg/runtime/sys_openbsd_amd64.s#newcode140 src/pkg/runtime/sys_openbsd_amd64.s:140: MOVQ 8(SP), AX // sec s/MOVQ/MOVL/ https://codereview.appspot.com/6820120/diff/2009/src/pkg/runtime/sys_openbsd_amd64.s#newcode141 src/pkg/runtime/sys_openbsd_amd64.s:141: MOVL ...
12 years, 5 months ago (2012-11-11 16:21:33 UTC) #1
jsing
The following comment should have been included: On OpenBSD time_t is an int - sys_gettimeofday ...
12 years, 5 months ago (2012-11-11 16:24:06 UTC) #2
minux1
On Mon, Nov 12, 2012 at 12:24 AM, Joel Sing <jsing@google.com> wrote: > On OpenBSD ...
12 years, 5 months ago (2012-11-11 16:46:13 UTC) #3
minux1
Hello jsing@google.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
12 years, 4 months ago (2012-12-13 10:42:31 UTC) #4
dave_cheney.net
LGTM once you feel you have tested it on enough systems. pandaboard(~/go/src/pkg/time) % ~/go/misc/benchcmp {old,new}.txt ...
12 years, 4 months ago (2012-12-13 22:16:28 UTC) #5
minux1
I've tested almost all the systems myself (I don't have *BSD/386 VM available, which means ...
12 years, 4 months ago (2012-12-13 22:27:07 UTC) #6
minux1
On 2012/12/13 22:16:28, dfc wrote: > pandaboard(~/go/src/pkg/time) % ~/go/misc/benchcmp {old,new}.txt > benchmark old ns/op new ...
12 years, 4 months ago (2012-12-13 22:27:46 UTC) #7
minux1
PTAL. Also updated time section of doc/go1.1.html.
12 years, 4 months ago (2012-12-13 22:33:34 UTC) #8
rsc
LGTM https://codereview.appspot.com/6820120/diff/25002/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/6820120/diff/25002/doc/go1.1.html#newcode90 doc/go1.1.html:90: On Darwin, FreeBSD, Linux, NetBSD and OpenBSD, previous ...
12 years, 4 months ago (2012-12-16 23:58:06 UTC) #9
minux1
https://codereview.appspot.com/6820120/diff/25002/doc/go1.1.html File doc/go1.1.html (right): https://codereview.appspot.com/6820120/diff/25002/doc/go1.1.html#newcode90 doc/go1.1.html:90: On Darwin, FreeBSD, Linux, NetBSD and OpenBSD, previous versions ...
12 years, 4 months ago (2012-12-17 14:39:28 UTC) #10
rsc
You're right, the nanosecond time for Darwin didn't happen until after Go 1. changeset: 13429:0b5426350053 ...
12 years, 4 months ago (2012-12-17 15:44:17 UTC) #11
jsing
LGTM https://codereview.appspot.com/6820120/diff/34001/src/pkg/runtime/sys_netbsd_386.s File src/pkg/runtime/sys_netbsd_386.s (right): https://codereview.appspot.com/6820120/diff/34001/src/pkg/runtime/sys_netbsd_386.s#newcode111 src/pkg/runtime/sys_netbsd_386.s:111: MOVL 20(SP), BX // nsec - should not ...
12 years, 4 months ago (2012-12-18 14:45:36 UTC) #12
minux1
12 years, 4 months ago (2012-12-18 14:57:46 UTC) #13
*** Submitted as https://code.google.com/p/go/source/detail?r=6625c3cb4d0e ***

runtime: use clock_gettime to get ns resolution for time.now & runtime.nanotime

For Linux/{386,arm}, FreeBSD/{386,amd64,arm}, NetBSD/{386,amd64},
OpenBSD/{386,amd64}.
Note: our Darwin implementation already has ns resolution.

Linux/386 (Core i7-2600 @ 3.40GHz, kernel 3.5.2-gentoo)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow          110          118   +7.27%

Linux/ARM (ARM Cortex-A8 @ 800MHz, kernel 2.6.32.28 android)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow          625          542  -13.28%

Linux/ARM (ARM Cortex-A9 @ 1GHz, Pandaboard)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow          992          909   -8.37%

FreeBSD 9-REL-p1/amd64 (Dell R610 Server with Xeon X5650 @ 2.67GHz)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow          699          695   -0.57%

FreeBSD 9-REL-p1/amd64 (Atom D525 @ 1.80GHz)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow         1553         1658   +6.76%

OpenBSD/amd64 (Dell E6410 with i5 CPU M 540 @ 2.53GHz)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow         1262         1236   -2.06%

OpenBSD/i386 (Asus eeePC 701 with Intel Celeron M 900MHz - locked to 631MHz)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow         5089         5043   -0.90%

NetBSD/i386 (VMware VM with Core i5 CPU @ 2.7GHz)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow          277          278   +0.36%

NetBSD/amd64 (VMware VM with Core i5 CPU @ 2.7Ghz)
benchmark       old ns/op    new ns/op    delta
BenchmarkNow          103          105   +1.94%


Thanks Maxim Khitrov, Joel Sing, and Dave Cheney for providing benchmark data.

R=jsing, dave, rsc
CC=golang-dev
https://codereview.appspot.com/6820120

https://codereview.appspot.com/6820120/diff/34001/src/pkg/runtime/sys_netbsd_...
File src/pkg/runtime/sys_netbsd_386.s (right):

https://codereview.appspot.com/6820120/diff/34001/src/pkg/runtime/sys_netbsd_...
src/pkg/runtime/sys_netbsd_386.s:111: MOVL	20(SP), BX		// nsec - should not
exceed 999999999
On 2012/12/18 14:45:37, jsing wrote:
> The "should not exceed" comment could go since we are no longer multiplying
the
> result.
Done
Sign in to reply to this message.

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