code review 5327063: runtime: add nanotime for Plan 9

runtime: add nanotime for Plan 9

[ality, 2011-11-03 00:40:59]

Hello paulzhol@gmail.com, rsc@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[paulzhol, 2011-11-03 08:26:39]

Is it a good idea to replicate syscall.Gettimeofday and syscall.DecodeBintime in
C ? Also I couldn't find any users for runtime.gettime.

Maybe you could use Tos.clock or Tos.pcycles (by declaring 'extern Tos *_tos;'
runtime/plan9/thread.c already uses it to read it's pid).

[dave_cheney.net, 2011-11-03 10:06:01]

Please do not use the RDTSC instruction as a proxy for gettimeofday. It will not
work in a multi core world. 

Sent from my iPhone

On 03/11/2011, at 20:22, Anthony Martin <ality@pbrane.org> wrote:

> paulzhol@gmail.com once said:
>> Is it a good idea to replicate syscall.Gettimeofday
>> and syscall.DecodeBintime in C ? Also I couldn't find
>> any users for runtime.gettime.
> 
> src/pkg/runtime/runtime.c:/nanotime is the only
> function that calls gettime.  Nanotime is used
> for the GC stats and Russ has a pending CL that
> changes time.Nanosleep to use runtime·nanotime
> directly.  My plan is to remove Gettimeofday
> from the syscall package after that CL goes in.
> 
> 
>> Maybe you could use Tos.clock or Tos.pcycles (by
>> declaring 'extern Tos *_tos;' runtime/plan9/thread.c
>> already uses it to read it's pid).
> 
> Neither of those will work but I could probably
> use tos.cyclefreq in combination with the RDTSC
> instruction although I would need some way of
> getting the epoch offset.  Thoughts?
> 
> Thanks,
>  Anthony

[paulzhol, 2011-11-03 13:26:22]

On Thu, Nov 3, 2011 at 11:22 AM, Anthony Martin <ality@pbrane.org> wrote:
> Neither of those will work but I could probably
> use tos.cyclefreq in combination with the RDTSC
> instruction although I would need some way of
> getting the epoch offset.  Thoughts?

Windows' gettime uses QueryPerformanceCounter (I'm not sure, but it is
sounds like the equiv. to RDTSC) without calculating epoch, only for
delta as hector noted.
Can't we do the same with tos->pcycles and ask Russ to move Nanotime
to syscall ?

How frequent runtime.gc gets invoked ? Maybe you could cache the file
descriptor of /dev/bintime to save the open syscall - it was on my
TODO for syscall.Gettimeofday.

Pavel

[rsc, 2011-11-03 13:36:36]

On Thu, Nov 3, 2011 at 09:26, Pavel Zholkover <paulzhol@gmail.com> wrote:
> How frequent runtime.gc gets invoked ? Maybe you could cache the file
> descriptor of /dev/bintime to save the open syscall - it was on my
> TODO for syscall.Gettimeofday.

I plan to use this as the implementation of time.Nanoseconds.
It needs to be correct.

Russ

[ality, 2011-11-03 22:52:42]

Hello paulzhol@gmail.com, rsc@golang.org, dave@cheney.net, rminnich@gmail.com
(cc: golang-dev@googlegroups.com),

Please take another look.

[paulzhol, 2011-11-04 11:00:56]

LGTM

[rsc, 2011-11-07 16:35:51]

Looks great except for needing the cache.
Without the cache nanotime is too expensive
to give a reliable answer.

Thanks for cleaning this up.
Russ

http://codereview.appspot.com/5327063/diff/7002/src/pkg/runtime/plan9/thread.c
File src/pkg/runtime/plan9/thread.c (right):

http://codereview.appspot.com/5327063/diff/7002/src/pkg/runtime/plan9/thread....
src/pkg/runtime/plan9/thread.c:79: fd = runtime·open((byte*)"/dev/bintime",
OREAD|OCEXEC);
This definitely needs to be cached.  If you throw a static int32 fd and a static
Lock here, it should be fine.  The OCEXEC avoids hurting fork+exec.

[ality, 2011-11-08 10:43:51]

Hello paulzhol@gmail.com, rsc@golang.org, dave@cheney.net, rminnich@gmail.com
(cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-11-08 20:20:52]

http://codereview.appspot.com/5327063/diff/15001/src/pkg/runtime/plan9/386/sys.s
File src/pkg/runtime/plan9/386/sys.s (right):

http://codereview.appspot.com/5327063/diff/15001/src/pkg/runtime/plan9/386/sy...
src/pkg/runtime/plan9/386/sys.s:91: // BUG(ality): vx32 doesn't decode PUSHA
If you'd like to make this vx32-friendly, the only
register that needs saving is SI, so you could use

PUSHL SI
CALL emptyfunc
POPL SI

http://codereview.appspot.com/5327063/diff/15001/src/pkg/runtime/plan9/thread.c
File src/pkg/runtime/plan9/thread.c (right):

http://codereview.appspot.com/5327063/diff/15001/src/pkg/runtime/plan9/thread...
src/pkg/runtime/plan9/thread.c:88: runtime·lock(&lk);
I wouldn't even bother with the lock, honestly.
If the file gets opened twice it is not a big deal.
The lock is not worth the cost.
Also I wouldn't bother trying to reopen the fd.
if pread fails, return 0.

[ality, 2011-11-08 20:53:31]

Hello paulzhol@gmail.com, rsc@golang.org, dave@cheney.net, rminnich@gmail.com
(cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2011-11-08 20:54:58]

http://codereview.appspot.com/5327063/diff/22001/src/pkg/runtime/plan9/thread.c
File src/pkg/runtime/plan9/thread.c (right):

http://codereview.appspot.com/5327063/diff/22001/src/pkg/runtime/plan9/thread...
src/pkg/runtime/plan9/thread.c:91: // TODO(ality): fast, accurate clocks on Plan
9 or Nix
I don't understand the TODO.  /dev/bintime is fast and accurate.

[rsc, 2011-11-18 02:47:22]

LGTM

Is the ten microseconds on a real machine or on vx32?

[rsc, 2011-11-18 03:09:36]

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