code review 5297078: time: faster Nanoseconds call

time: faster Nanoseconds call

runtime knows how to get the time of day
without allocating memory.

[rsc, 2011-11-02 21:33:06]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[dsymonds, 2011-11-02 21:37:57]

LGTM

Seems odd to put the C code for pkg time in the runtime dir, though.

[dave_cheney.net, 2011-11-02 21:46:26]

This is a great improvement. It will remove one allocation in the fast
path on net.Conn.Read/Write. Why is it necessary to FLUSH() the return
value, can runtime.nanotime cause a scheduling event ?

On Thu, Nov 3, 2011 at 8:37 AM, David Symonds <dsymonds@golang.org> wrote:
> LGTM
>
> Seems odd to put the C code for pkg time in the runtime dir, though.
>

[rsc, 2011-11-02 21:49:24]

On Wed, Nov 2, 2011 at 17:46, Dave Cheney <dave@cheney.net> wrote:
> This is a great improvement. It will remove one allocation in the fast
> path on net.Conn.Read/Write. Why is it necessary to FLUSH() the return
> value, can runtime.nanotime cause a scheduling event ?

FLUSH tricks the C compiler into writing the value
to the slot on the stack where Go expects to find
the return value.

[dave_cheney.net, 2011-11-02 21:51:53]

Thanks for explanation, I was thinking that FLUSH caused a memory fence.

On Thu, Nov 3, 2011 at 8:49 AM, Russ Cox <rsc@golang.org> wrote:
> On Wed, Nov 2, 2011 at 17:46, Dave Cheney <dave@cheney.net> wrote:
>> This is a great improvement. It will remove one allocation in the fast
>> path on net.Conn.Read/Write. Why is it necessary to FLUSH() the return
>> value, can runtime.nanotime cause a scheduling event ?
>
> FLUSH tricks the C compiler into writing the value
> to the slot on the stack where Go expects to find
> the return value.
>

[hector, 2011-11-02 22:48:26]

This won't work on Windows because gettime doesn't actually return the current
time - it's only good for getting time deltas.

[r, 2011-11-02 22:50:47]

http://codereview.appspot.com/5297078/diff/1001/src/pkg/runtime/runtime.c
File src/pkg/runtime/runtime.c (right):

http://codereview.appspot.com/5297078/diff/1001/src/pkg/runtime/runtime.c#new...
src/pkg/runtime/runtime.c:670: time·Nanoseconds(int64 ret)
i'd be happier if this were in a different .c file, called time.c

[rsc, 2011-11-03 16:08:57]

PTAL

I moved the time stuff into time.goc, which is the
canonical way to provide some stuff for another package
from runtime.

I also dropped gettime and converted the assembly for
Darwin, FreeBSD, Linux, and OpenBSD to be nanotime.

I hope someone will tell me what Windows should look like.

Russ

[hector, 2011-11-03 16:56:13]

GetSystemTimeAsFileTime is what currently provides the time to time.Nanoseconds.
 The FileTime is then converted with func (ft *Filetime) Nanoseconds() int64 in
syscall/ztypes_windows.go.

[hector, 2011-11-03 16:58:59]

http://codereview.appspot.com/5297078/diff/5/src/pkg/runtime/windows/thread.c
File src/pkg/runtime/windows/thread.c (left):

http://codereview.appspot.com/5297078/diff/5/src/pkg/runtime/windows/thread.c...
src/pkg/runtime/windows/thread.c:205:
runtime·stdcall(runtime·QueryPerformanceCounter, 1, &count);
References to QueryPerformanceCounter and QueryPerformanceFrequency should be
removed from the top of this file.

[cw, 2011-11-03 17:06:44]

Before: 151 ns/call
After: 44ns/call

LGTM!

[r, 2011-11-03 17:35:02]

LGTM

http://codereview.appspot.com/5297078/diff/5/src/pkg/runtime/plan9/386/signal.c
File src/pkg/runtime/plan9/386/signal.c (right):

http://codereview.appspot.com/5297078/diff/5/src/pkg/runtime/plan9/386/signal...
src/pkg/runtime/plan9/386/signal.c:10: return 0;
// TODO?

http://codereview.appspot.com/5297078/diff/5/src/pkg/runtime/windows/thread.c
File src/pkg/runtime/windows/thread.c (right):

http://codereview.appspot.com/5297078/diff/5/src/pkg/runtime/windows/thread.c...
src/pkg/runtime/windows/thread.c:203: return 0;
// TODO?

http://codereview.appspot.com/5297078/diff/5/src/pkg/time/sys.go
File src/pkg/time/sys.go (right):

http://codereview.appspot.com/5297078/diff/5/src/pkg/time/sys.go#newcode13
src/pkg/time/sys.go:13: // Nanoseconds is implemented by runtime
s/runtime/package runtime./

[rsc, 2011-11-03 19:33:02]

Hector, PTAL at the Windows changes.

Now calling GetSystemTimeAsFileTime.
There's a name I haven't heard in a decade.

Russ

[hector, 2011-11-03 20:14:08]

Windows changes LGTM.

[rsc, 2011-11-03 21:35:32]

*** Submitted as http://code.google.com/p/go/source/detail?r=c8c9ccd19020 ***

time: faster Nanoseconds call

runtime knows how to get the time of day
without allocating memory.

R=golang-dev, dsymonds, dave, hectorchu, r, cw
CC=golang-dev
http://codereview.appspot.com/5297078