code review 2321043: time: add After

time: add After

Permits one to easily put a timeout in a select:

select {
case <-ch:
	// foo
case <-time.After(1e6):
	// bar
}

[adg, 2010-10-05 03:43:24]

Hello r (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[r2, 2010-10-05 03:59:55]

I don't mind the idea but the name isn't right.   Timeout describes to the
example you gave but not to what the function does.  It's more like Delay or
maybe even DelayedSend.  I can't think of a good short name, but maybe Delay is
the best compromise.

And although this function should probably live, while you're there you might
want a niladic version
	DelayFn(ns int64, fn func())

I'd like Russ's opinion too. This is nice but it's an API addition with
stylistic consequences.

-rob

[rog, 2010-10-05 10:03:38]

On 5 October 2010 04:43,  <adg@golang.org> wrote:
> +func Timeout(ns int64) <-chan int64 {
> +       ch := make(chan int64, 1)
> +       go func() {
> +               Sleep(ns)
> +               ch <- ns

i wonder if it's actually useful sending the delay
duration down the channel - it doesn't tell you
anything you didn't already know.

one possibility would be to send the actual
time slept for, or the absolute time when
the Sleep finished.

if one is calculating timeouts, it's often convenient
to work in terms of absolute times - occam's
timeout primitive was AFTER: send a value
on a channel after a particular absolute time,
which may be a more convenient primitive,
although i'm not sure what a good Go name
for it might be.

it's also potentially more accurate, because it
can take into account the time taken to start
the goroutine, which may be arbitrarily delayed
by the scheduler.

> +       }()
> +       return ch
> +}
> Index: src/pkg/time/sleep_test.go
> ===================================================================
> --- a/src/pkg/time/sleep_test.go
> +++ b/src/pkg/time/sleep_test.go
> @@ -24,3 +24,14 @@
>                t.Fatalf("Sleep(%d) slept for only %d ns", delay, duration)
>        }
>  }
> +
> +func TestTimeout(t *testing.T) {
> +       const delay = int64(100e6)
> +       start := Nanoseconds()
> +       ch := Timeout(delay)
> +       <-ch
> +       duration := Nanoseconds() - start
> +       if duration < delay {
> +               t.Fatalf("Timeout(%d) slept for only %d ns", delay,
> duration)
> +       }
> +}
>
>
>

[rsc, 2010-10-05 10:15:54]

i'd go with time.Delay.

rog's comment about using absolute time
sounds fine for the implementation but
not worth doing in the api.

russ

[adg, 2010-10-05 10:17:35]

Is there an overhead to calling time.Nanoseconds() ? Should that be a concern?

On 5 October 2010 21:15, Russ Cox <rsc@golang.org> wrote:
> i'd go with time.Delay.
>
> rog's comment about using absolute time
> sounds fine for the implementation but
> not worth doing in the api.
>
> russ
>

[Sameer at Google, 2010-10-05 11:03:19]

On 2010/10/05 10:15:54, rsc wrote:
> i'd go with time.Delay.

time.After(ns int64, f func() interface{}) reads well:
  when := <-time.After(300e6, func() interface{} { return time.Nanoseconds() }):
  fmt.Printf("Function actually ran at %v", when)
RunAfter() would also work, though more verbose.

> 
> rog's comment about using absolute time
> sounds fine for the implementation but
> not worth doing in the api.
> 
> russ

[r2, 2010-10-05 11:54:39]

I like After too.

-rob

On Tuesday, October 5, 2010,  <sameer@google.com> wrote:
> On 2010/10/05 10:15:54, rsc wrote:
>
> i'd go with time.Delay.
>
>
> time.After(ns int64, f func() interface{}) reads well:
>   when := <-time.After(300e6, func() interface{} { return
> time.Nanoseconds() }):
>   fmt.Printf("Function actually ran at %v", when)
> RunAfter() would also work, though more verbose.
>
>
>
> rog's comment about using absolute time
> sounds fine for the implementation but
> not worth doing in the api.
>
>
>
> russ
>
>
>
>
> http://codereview.appspot.com/2321043/
>

[rog, 2010-10-05 12:36:04]

On 5 October 2010 11:16, Andrew Gerrand <adg@golang.org> wrote:
> Is there an overhead to calling time.Nanoseconds() ? Should that be a concern?

AFAIR there is currently a significant overhead to calling time.Nanoseconds()
as it is treated as a potentially blocking syscall. it's not too hard
to remove that overhead if desired.

the other concern i have about this interface/implementation
is that it lends itself very easily to loops like this:

func GetData(data <-chan []byte) os.Error {
    for {
        select {
        case x := <-data:
            if closed(data) {
                return nil
            }
            doSomething(x)
        case time.Timeout(30 * 1e9):
            // time out if no data received for 30s
            return os.ErrorString("timeout")
        }
    }
}

this seems perfectly natural and harmless but, given
any significant number of messages on data, a
very large amount of garbage will be generated.

perhaps the intention is to assume a GC that can clear up
garbage goroutines, but i'd prefer to have an interface that
can work now.

when i've needed a timeout in the past, i've done
something like:

for {
        t := time.NewTicker(DelayTime)
        select {
        case x := <-data:
                doSomething(x)
        case <-t.C:
                t.Stop()
                return os.ErrorString("timeout")
        }
        t.Stop()
}

this has significantly more overhead, but at least it
can't generate pathological quantities of garbage.

in the longer term, perhaps it might be worth building
time into the runtime support?

[rsc, 2010-10-05 13:17:41]

Ian fixed time.Nanoseconds.
Or if he didn't we can (but I think he did).
I don't believe that should factor into the API.

I'm not convinced about time.After(t, f).

    x := time.After(t, func()interface{}{return math.Sqrt(2)}).(float64)

seems clunkier than

    time.SleepUntil(t)
    x := math.Sqrt(2)

Russ

[r2, 2010-10-05 13:40:57]

On Oct 5, 2010, at 3:17 PM, Russ Cox wrote:

> Ian fixed time.Nanoseconds.
> Or if he didn't we can (but I think he did).
> I don't believe that should factor into the API.
> 
> I'm not convinced about time.After(t, f).
> 
>    x := time.After(t, func()interface{}{return math.Sqrt(2)}).(float64)
> 
> seems clunkier than
> 
>    time.SleepUntil(t)
>    x := math.Sqrt(2)
> 
> Russ

fair enough, but

   x := time.After(t, func()interface{}{return math.Sqrt(2)}).(float64)

seems clunkier than

   time.After(t, func(){x = math.Sqrt(2)})

which is closer to what i had in mind.

if t is absolute time, the word is At not After.  at 10pm do this.  after 30
seconds, do that.

-rob

[rog, 2010-10-05 13:51:17]

On 5 October 2010 14:40, Rob 'Commander' Pike <r@google.com> wrote:
> if t is absolute time, the word is At not After.  at 10pm do this.  after 30
seconds, do that.

At would be right if the action was guaranteed to take place at the that
exact time; but it's not - the action will take place some time after
the asked-for time because of vagaries in the time keeping.
that, i believe, was the reasoning behind the naming of the Occam
primitive. but perhaps this level of precision doesn't matter
so much when there's so much jitter behind the scenes.

as far as usage goes, "after 10pm" works for me just as well as "after
30 seconds"

[r2, 2010-10-05 13:54:46]

On Oct 5, 2010, at 3:51 PM, roger peppe wrote:
> 
> 
> as far as usage goes, "after 10pm" works for me just as well as "after
> 30 seconds"

but that's the problem. my point is in the opposite direction.  "at 10pm" is
clear that it's about an absolute time.  "at 30 seconds" is close to
meaningless.  so if it's an absolute time, "at" is clearer and "after" is
confusing because it's unclear whether the time value is absolute or relative.

-rob

[rog, 2010-10-05 14:05:04]

On 5 October 2010 14:54, Rob 'Commander' Pike <r@google.com> wrote:
>
> On Oct 5, 2010, at 3:51 PM, roger peppe wrote:
>
> as far as usage goes, "after 10pm" works for me just as well as "after
> 30 seconds"
>
> but that's the problem. my point is in the opposite direction.  "at 10pm" is
> clear that it's about an absolute time.  "at 30 seconds" is close to
> meaningless.  so if it's an absolute time, "at" is clearer and "after" is
> confusing because it's unclear whether the time value is absolute or
> relative.
> -rob
>

agreed.

but in general, i don't think having a function that runs a function after
a given amount of time is that useful, as it's trivial to implement
with the current primitives.

it is harder to implement a decent timeout for select, which
was the original motivation for this patch. i still don't see
a good solution.

[rog, 2010-10-05 14:18:50]

On 5 October 2010 14:17, Russ Cox <rsc@golang.org> wrote:
> Ian fixed time.Nanoseconds.
> Or if he didn't we can (but I think he did).

BTW, it doesn't look to me as if it's fixed.

time.Nanoseconds calls os.Time which calls
syscall.Gettimeofday which calls syscall.gettimeofday
which calls Syscall which calls runtime.entersyscall and
runtime.exitsyscall which can call the GC, scheduler, etc.

i only looked at amd64 - others may be different.

[peterGo, 2010-10-05 14:40:16]

Roger,

> > Ian fixed time.Nanoseconds.
> > Or if he didn't we can (but I think he did).
> 
> BTW, it doesn't look to me as if it's fixed.

Are  you saying this CL doesn't work?

push by i...@golang.org - syscall: Use vsyscall for syscall.Gettimeofday and
.Time on linux amd6... on 2010-09-21 13:50 GMT - golang-checkins | Google Groups
http://groups.google.com/group/golang-checkins/browse_thread/thread/f681524f4...

Peter

On 2010/10/05 14:18:50, rog wrote:
> On 5 October 2010 14:17, Russ Cox <mailto:rsc@golang.org> wrote:
> > Ian fixed time.Nanoseconds.
> > Or if he didn't we can (but I think he did).
> 
> BTW, it doesn't look to me as if it's fixed.
> 
> time.Nanoseconds calls os.Time which calls
> syscall.Gettimeofday which calls syscall.gettimeofday
> which calls Syscall which calls runtime.entersyscall and
> runtime.exitsyscall which can call the GC, scheduler, etc.
> 
> i only looked at amd64 - others may be different.

[rog, 2010-10-05 14:46:25]

ah, linux only.

assuming that the fix would have been applied universally,
i only looked at the platform i'm using (darwin).

On 5 October 2010 15:40,  <go.peter.90@gmail.com> wrote:
> Roger,
>
>> > Ian fixed time.Nanoseconds.
>> > Or if he didn't we can (but I think he did).
>
>> BTW, it doesn't look to me as if it's fixed.
>
> Are  you saying this CL doesn't work?
>
> push by i...@golang.org - syscall: Use vsyscall for syscall.Gettimeofday
> and .Time on linux amd6... on 2010-09-21 13:50 GMT - golang-checkins |
> Google Groups
>
http://groups.google.com/group/golang-checkins/browse_thread/thread/f681524f4...
>
> Peter
>
> On 2010/10/05 14:18:50, rog wrote:
>>
>> On 5 October 2010 14:17, Russ Cox <mailto:rsc@golang.org> wrote:
>> > Ian fixed time.Nanoseconds.
>> > Or if he didn't we can (but I think he did).
>
>> BTW, it doesn't look to me as if it's fixed.
>
>> time.Nanoseconds calls os.Time which calls
>> syscall.Gettimeofday which calls syscall.gettimeofday
>> which calls Syscall which calls runtime.entersyscall and
>> runtime.exitsyscall which can call the GC, scheduler, etc.
>
>> i only looked at amd64 - others may be different.
>
>
>
> http://codereview.appspot.com/2321043/
>

[iant2, 2010-10-05 15:09:16]

roger peppe <rogpeppe@gmail.com> writes:

> On 5 October 2010 14:17, Russ Cox <rsc@golang.org> wrote:
>> Ian fixed time.Nanoseconds.
>> Or if he didn't we can (but I think he did).
>
> BTW, it doesn't look to me as if it's fixed.
>
> time.Nanoseconds calls os.Time which calls
> syscall.Gettimeofday which calls syscall.gettimeofday
> which calls Syscall which calls runtime.entersyscall and
> runtime.exitsyscall which can call the GC, scheduler, etc.
>
> i only looked at amd64 - others may be different.

I find time.Nanoseconds for AMD64 GNU/Linux only, because that is the
only target which uses vsyscalls.

However, it can in general be fixed for all targets.  I think we just
need to frob the syscall support to declare some syscalls as
nonblocking, and have the generated code for those syscalls call
Rawsyscall rather than Syscall.

Ian

[rog, 2010-10-05 16:39:20]

On 5 October 2010 16:09, Ian Lance Taylor <iant@google.com> wrote:
> roger peppe <rogpeppe@gmail.com> writes:
>
>> On 5 October 2010 14:17, Russ Cox <rsc@golang.org> wrote:
>>> Ian fixed time.Nanoseconds.
>>> Or if he didn't we can (but I think he did).
>>
>> BTW, it doesn't look to me as if it's fixed.
>>
>> time.Nanoseconds calls os.Time which calls
>> syscall.Gettimeofday which calls syscall.gettimeofday
>> which calls Syscall which calls runtime.entersyscall and
>> runtime.exitsyscall which can call the GC, scheduler, etc.
>>
>> i only looked at amd64 - others may be different.
>
> I find time.Nanoseconds for AMD64 GNU/Linux only, because that is the
> only target which uses vsyscalls.
>
> However, it can in general be fixed for all targets.  I think we just
> need to frob the syscall support to declare some syscalls as
> nonblocking, and have the generated code for those syscalls call
> Rawsyscall rather than Syscall.

yes, that's the solution i was expecting to find.

[nigeltao_gnome, 2010-10-06 01:17:26]

On 5 October 2010 23:36, roger peppe <rogpeppe@gmail.com> wrote:
> when i've needed a timeout in the past, i've done
> something like:
>
> for {
>        t := time.NewTicker(DelayTime)
>        select {
>        case x := <-data:
>                doSomething(x)
>        case <-t.C:
>                t.Stop()
>                return os.ErrorString("timeout")
>        }
>        t.Stop()
> }
>
> this has significantly more overhead, but at least it
> can't generate pathological quantities of garbage.

I may be misunderstanding you, but I think calling time.NewTicker in
every iteration of the loop is strictly more complicated than calling
time.Timeout (or After) every iteration of the loop. The Timeout
implemention writes to a buffered channel, so doesn't block, and exits
cleanly regardless of which branch the select takes.

In my own applications I've had one timing goroutine for the entire
for loop, but not a new one per iteration like I think you're
suggesting.

[Sameer at Google, 2010-10-06 02:24:45]

On 2010/10/05 14:05:04, rog wrote:
> On 5 October 2010 14:54, Rob 'Commander' Pike <mailto:r@google.com> wrote:
> >
> > On Oct 5, 2010, at 3:51 PM, roger peppe wrote:
> >
> > as far as usage goes, "after 10pm" works for me just as well as "after
> > 30 seconds"
> >
> > but that's the problem. my point is in the opposite direction. &nbsp;"at
10pm" is
> > clear that it's about an absolute time. &nbsp;"at 30 seconds" is close to
> > meaningless. &nbsp;so if it's an absolute time, "at" is clearer and "after"
is
> > confusing because it's unclear whether the time value is absolute or
> > relative.
> > -rob
> >
> 
> agreed.
> 
> but in general, i don't think having a function that runs a function after
> a given amount of time is that useful, as it's trivial to implement
> with the current primitives.
> 
> it is harder to implement a decent timeout for select, which
> was the original motivation for this patch. i still don't see
> a good solution.

Agreed.  Andrew's latest blog post points out some subtleties in getting a
timeout for select right (I had been using time.Tick(timeoutNS), which leaks). 
Whatever the result of this discussion, I'd like to have a simple and correct
way to write:
go func(){ results <- something() }()
timeout := ...(300e6)...  // 300 ms
select {
case result:= <-results:  // handle success
case <-timeout: // handle timeout
}

[adg, 2010-10-06 02:28:30]

Hello r, r2, rog, rsc, sameer1, PeterGo, iant2, nigeltao_gnome (cc:
golang-dev@googlegroups.com),

Please take another look.

[Sameer at Google, 2010-10-06 02:36:02]

This interface works for my use cases.

http://codereview.appspot.com/2321043/diff/20001/src/pkg/time/sleep_test.go
File src/pkg/time/sleep_test.go (right):

http://codereview.appspot.com/2321043/diff/20001/src/pkg/time/sleep_test.go#n...
src/pkg/time/sleep_test.go:32: <-ch
In addition to the duration check below, this should also hold:

end := <-ch
diff := end - start
if diff < delay {
  t.Fatalf("After(%d)'s result was only %s ns after start", delay, diff)
}

[adg, 2010-10-06 03:49:44]

On 6 October 2010 13:36,  <sameer@google.com> wrote:
>
http://codereview.appspot.com/2321043/diff/20001/src/pkg/time/sleep_test.go#n...
> src/pkg/time/sleep_test.go:32: <-ch
> In addition to the duration check below, this should also hold:
>
> end := <-ch
> diff := end - start
> if diff < delay {
>  t.Fatalf("After(%d)'s result was only %s ns after start", delay, diff)
> }

Done.

[r, 2010-10-06 06:02:31]

i remain nervous about how it will be used but we can continue that discussion

http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go
File src/pkg/time/sleep.go (right):

http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go#newcode35
src/pkg/time/sleep.go:35: for t < end && err == nil {
this is pretty subtle and i think worth a comment

http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go#newcode44
src/pkg/time/sleep.go:44: // sysSleep makes the Sleep syscall.
"makes" is an odd word.  anyway what this does is wrap Sleep to give you an
os.Error back

[rog, 2010-10-06 10:07:55]

http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go
File src/pkg/time/sleep.go (right):

http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go#newcode28
src/pkg/time/sleep.go:28: // on the returned channel.
i think it would be worth having the sent value be the same kind of thing as the
argument value. i.e. if After is waiting a particular duration, then it should
return the duration waited for.

on balance, i still think i'd prefer At, as it's potentially more accurate (if
the caller needs a near-exact end time, then After is only estimating it,
whereas the caller knows it) and After is trivially buildable with At.

[rsc1, 2010-10-06 10:19:35]

http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go
File src/pkg/time/sleep.go (right):

http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go#newcode29
src/pkg/time/sleep.go:29: func After(ns int64) <-chan int64 {
After is a preposition: after x, do y.  After(t, f).
I still think this is Delay.

http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go#newcode44
src/pkg/time/sleep.go:44: // sysSleep makes the Sleep syscall.
On 2010/10/06 06:02:31, r wrote:
> "makes" is an odd word.  anyway what this does is wrap Sleep to give you an
> os.Error back

func sleep.

also, make it deal with the interrupts here
and then the two loops above go away.

[r2, 2010-10-06 10:25:09]

On Oct 6, 2010, at 12:19 PM, rsc@google.com wrote:

> 
> http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go
> File src/pkg/time/sleep.go (right):
> 
>
http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go#newcode29
> src/pkg/time/sleep.go:29: func After(ns int64) <-chan int64 {
> After is a preposition: after x, do y.  After(t, f).
> I still think this is Delay.

I had the same opinion until I saw how it looks in code.

	t := <-time.After(x)

that reads well.

-rob

[r2, 2010-10-06 10:26:42]

On Oct 6, 2010, at 12:25 PM, Rob 'Commander' Pike wrote:

> 
> On Oct 6, 2010, at 12:19 PM, rsc@google.com wrote:
> 
>> 
>> http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go
>> File src/pkg/time/sleep.go (right):
>> 
>>
http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go#newcode29
>> src/pkg/time/sleep.go:29: func After(ns int64) <-chan int64 {
>> After is a preposition: after x, do y.  After(t, f).
>> I still think this is Delay.
> 
> I had the same opinion until I saw how it looks in code.
> 
> 	t := <-time.After(x)
> 
> that reads well.
> 
> -rob

that's also an argument against rog's complaint about absolute/relative result.

-rob

[adg, 2010-10-06 10:28:46]

On 6 October 2010 21:19,  <rsc@google.com> wrote:
>
http://codereview.appspot.com/2321043/diff/24001/src/pkg/time/sleep.go#newcode29
> src/pkg/time/sleep.go:29: func After(ns int64) <-chan int64 {
> After is a preposition: after x, do y.  After(t, f).
> I still think this is Delay.

Delay what? It doesn't seem much clearer.

How about 'SendAfter' or 'TickAfter'?

> also, make it deal with the interrupts here
> and then the two loops above go away.

The reason I didn't do that in the first place is that t is set to the
current time before launching the goroutine, making its end time more
likely to be accurate. Not worth it?

[rsc, 2010-10-06 10:34:55]

okay, After is fine

[rog, 2010-10-06 10:52:13]

> that's also an argument against rog's complaint about absolute/relative
> result.
> -rob

really?

for me, something like:

t := calculateNextTime()
actualTime := <-time.After(t)

would work well.

i.e. "wait until some time as soon as possible after t and tell me
when that was"

this is somewhat analogous to the index vs len debate for the
second argument to the slice operator.

using an absolute time means that it's trivial to do something at
regular intervals
without fear of drifting over time:

t := time.Nanoseconds() + 30e9
for {
    <-time.After(t)
    t += 30e9
}

it also occurs to me that an absolute time is actually more
natural to use in a select, because it makes it easy to service
other (non-timing-out) events in the select without
needing to alter the argument to time.After (or At, or whatever).

[Sameer at Google, 2010-10-06 12:38:55]

On Wed, Oct 6, 2010 at 6:52 AM, roger peppe <rogpeppe@gmail.com> wrote:

> > that's also an argument against rog's complaint about absolute/relative
> > result.
> > -rob
>
> really?
>
> for me, something like:
>
> t := calculateNextTime()
> actualTime := <-time.After(t)
>
> would work well.
>
> i.e. "wait until some time as soon as possible after t and tell me
> when that was"
>
> this is somewhat analogous to the index vs len debate for the
> second argument to the slice operator.
>
> using an absolute time means that it's trivial to do something at
> regular intervals
> without fear of drifting over time:
>
> t := time.Nanoseconds() + 30e9
> for {
>    <-time.After(t)
>    t += 30e9
> }
>

Isn't this what time.Ticker is for?


>
> it also occurs to me that an absolute time is actually more
> natural to use in a select, because it makes it easy to service
> other (non-timing-out) events in the select without
> needing to alter the argument to time.After (or At, or whatever).
>

FWIW, the C++ Bigtable APIs switched from relative to absolute deadlines
because the latter was clearer to clients and simpler to get right.

But I think either way can be coded cleanly without needing to alter the
arguments repeatedly:

timeout := time.After(time.Nanoseconds() + 1e9)  --or--  time.Delay(1e9)
 --or--  time.TickOnce(1e9)
for iHaveWorkToDo {
  select {
  case <-finishedSomeWork:
  case <-timeout:
  }
}

-- 
Sameer

[rog, 2010-10-06 13:11:10]

On 6 October 2010 13:38, Sameer Ajmani <sameer@google.com> wrote:
>> using an absolute time means that it's trivial to do something at
>> regular intervals
>> without fear of drifting over time:
>>
>> t := time.Nanoseconds() + 30e9
>> for {
>>    <-time.After(t)
>>    t += 30e9
>> }
>
> Isn't this what time.Ticker is for?

true.

>> it also occurs to me that an absolute time is actually more
>> natural to use in a select, because it makes it easy to service
>> other (non-timing-out) events in the select without
>> needing to alter the argument to time.After (or At, or whatever).
>
> FWIW, the C++ Bigtable APIs switched from relative to absolute deadlines
> because the latter was clearer to clients and simpler to get right.

i agree with this.

> But I think either way can be coded cleanly without needing to alter the
> arguments repeatedly:
>
> timeout := time.After(time.Nanoseconds() + 1e9)  --or--  time.Delay(1e9)
>  --or--  time.TickOnce(1e9)

my argument is that if you actually want an absolute deadline, say t,
then time.Delay(t - time.Nanoseconds()) is strictly less accurate than
time.After(t) because the former form loses information about the
actual desired deadline. in fact, the difference is almost certainly
negligible on current Go platforms so i suppose it all hinges on
how this facility will be mostly used.

[Sameer at Google, 2010-10-06 14:29:07]

On Wed, Oct 6, 2010 at 9:11 AM, roger peppe <rogpeppe@gmail.com> wrote:

> On 6 October 2010 13:38, Sameer Ajmani <sameer@google.com> wrote:
> >> using an absolute time means that it's trivial to do something at
> >> regular intervals
> >> without fear of drifting over time:
> >>
> >> t := time.Nanoseconds() + 30e9
> >> for {
> >>    <-time.After(t)
> >>    t += 30e9
> >> }
> >
> > Isn't this what time.Ticker is for?
>
> true.
>
> >> it also occurs to me that an absolute time is actually more
> >> natural to use in a select, because it makes it easy to service
> >> other (non-timing-out) events in the select without
> >> needing to alter the argument to time.After (or At, or whatever).
> >
> > FWIW, the C++ Bigtable APIs switched from relative to absolute deadlines
> > because the latter was clearer to clients and simpler to get right.
>
> i agree with this.
>
> > But I think either way can be coded cleanly without needing to alter the
> > arguments repeatedly:
> >
> > timeout := time.After(time.Nanoseconds() + 1e9)  --or--  time.Delay(1e9)
> >  --or--  time.TickOnce(1e9)
>
> my argument is that if you actually want an absolute deadline, say t,
> then time.Delay(t - time.Nanoseconds()) is strictly less accurate than
> time.After(t) because the former form loses information about the
> actual desired deadline. in fact, the difference is almost certainly
> negligible on current Go platforms so i suppose it all hinges on
> how this facility will be mostly used.
>

Either seems fine to me.  And I now agree that "After(t)" reads better when
"t" is an absolute time, whereas "Delay(t)" reads better when "t" is a
duration.  I'll make one last pitch for the general form:
  // If "f" is non-nil, runs v := f() after time "when" and sends v on the
returned channel.
  // If "f" is nil, sends nil on the returned channel after time "when".
  After(when int64, f func() interface{}) <-chan interface{}
-- 
Sameer

[rsc, 2010-10-06 14:34:39]

This is turning into a bikeshed discussion.
Let's cut it off at func After(t int64) <-chan int64.

Russ

[rog, 2010-10-06 15:18:54]

On 6 October 2010 15:34, Russ Cox <rsc@golang.org> wrote:
> This is turning into a bikeshed discussion.
> Let's cut it off at func After(t int64) <-chan int64.

ok, but is that with t as a duration or absolute?

[rsc, 2010-10-06 15:20:30]

On Wed, Oct 6, 2010 at 11:18, roger peppe <rogpeppe@gmail.com> wrote:
> On 6 October 2010 15:34, Russ Cox <rsc@golang.org> wrote:
>> This is turning into a bikeshed discussion.
>> Let's cut it off at func After(t int64) <-chan int64.
>
> ok, but is that with t as a duration or absolute?

it's a duration, like all the other functions that take ns int64.

russ

[adg, 2010-10-07 00:19:56]

Hello r, rog, rsc, sameer1, PeterGo, iant2, nigeltao_gnome, r2 (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc1, 2010-10-07 07:16:24]

LGTM

http://codereview.appspot.com/2321043/diff/38001/src/pkg/time/sleep.go
File src/pkg/time/sleep.go (right):

http://codereview.appspot.com/2321043/diff/38001/src/pkg/time/sleep.go#newcode13
src/pkg/time/sleep.go:13: // Higher resolution sleeping may be provided by
syscall.Nanosleep
This is such a broken comment.
Not for this CL but can we please just make sleep work
and delete the comment?

http://codereview.appspot.com/2321043/diff/38001/src/pkg/time/sleep_test.go
File src/pkg/time/sleep_test.go (right):

http://codereview.appspot.com/2321043/diff/38001/src/pkg/time/sleep_test.go#n...
src/pkg/time/sleep_test.go:32: end := <-ch
end := <-After(delay)

[adg, 2010-10-11 02:45:38]

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

time: add After

Permits one to easily put a timeout in a select:

select {
case <-ch:
	// foo
case <-time.After(1e6):
	// bar
}

R=r, rog, rsc, sameer1, PeterGo, iant, nigeltao_gnome
CC=golang-dev
http://codereview.appspot.com/2321043

Committer: Andrew Gerrand <adg@golang.org>