code review 73110043: runtime: support for fake time like on Go Playground

runtime: support for fake time like on Go Playground

Reapply CL 15690048 from the rsc-go13-nacl branch which was only partially applied during the previous merge.

Description taken from CL 15690048.

To enable, entry assembly should initialize timens to a nonzero start time value (nanoseconds since 1970), and assembly for time.now must use a nonzero timens in preference to finding out the real time.

[dave_cheney.net, 2014-03-09 02:20:03]

Hello golang-codereviews@googlegroups.com,

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

[remyoudompheng, 2014-03-09 09:10:55]

On 2014/03/09 02:20:03, dfc wrote:
> Hello mailto:golang-codereviews@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go

In the description: this patch doesn't contain "a deletion or two".

Also it looks like timejump is called even if runtime·timens is not zero. Does
it mean it will trigger even in a regular program e.g.

package main
import "time"
func main() { time.Sleep(500*time.Hour); }

?

[dvyukov, 2014-03-11 07:34:40]

https://codereview.appspot.com/73110043/diff/40001/src/pkg/runtime/proc.c
File src/pkg/runtime/proc.c (right):

https://codereview.appspot.com/73110043/diff/40001/src/pkg/runtime/proc.c#new...
src/pkg/runtime/proc.c:2510: if((gp = runtime·timejump()) != nil) {
this scares me

here is a lock order inversion between schedlock and timerlock, currently we
take timerlock->schedlock, and this adds schedlock->timerlock

it's possible that when timejump is executed, no other code is running
but it's unclear to me whether it's the case with e.g. external threads (extram)

there is also a possibility that timejump does not work with external threads at
all (never executed)

how general is this functionality intended to be?

[dave_cheney.net, 2014-03-11 07:56:36]

Thanks Dmitry and Remy for your reviews.

What I would like to do is abandon this change for the moment and
replace it with a smaller change that rolls back the fake time parts
that have already been comitted to the tree.

The goal is to getting to a buildable nacl/{386,amd64p32} trunk and I
think fake time support can be added after we have a working
builder(s).

How does that sound ?

On Tue, Mar 11, 2014 at 6:34 PM,  <dvyukov@google.com> wrote:
>
> https://codereview.appspot.com/73110043/diff/40001/src/pkg/runtime/proc.c
> File src/pkg/runtime/proc.c (right):
>
>
https://codereview.appspot.com/73110043/diff/40001/src/pkg/runtime/proc.c#new...
> src/pkg/runtime/proc.c:2510: if((gp = runtime·timejump()) != nil) {
> this scares me
>
> here is a lock order inversion between schedlock and timerlock,
> currently we take timerlock->schedlock, and this adds
> schedlock->timerlock
>
> it's possible that when timejump is executed, no other code is running
> but it's unclear to me whether it's the case with e.g. external threads
> (extram)
>
> there is also a possibility that timejump does not work with external
> threads at all (never executed)
>
> how general is this functionality intended to be?
>
> https://codereview.appspot.com/73110043/

[dvyukov, 2014-03-11 08:23:14]

On Tue, Mar 11, 2014 at 11:56 AM, Dave Cheney <dave@cheney.net> wrote:
> Thanks Dmitry and Remy for your reviews.
>
> What I would like to do is abandon this change for the moment and
> replace it with a smaller change that rolls back the fake time parts
> that have already been comitted to the tree.
>
> The goal is to getting to a buildable nacl/{386,amd64p32} trunk and I
> think fake time support can be added after we have a working
> builder(s).
>
> How does that sound ?

less changes to proc.c we have before the release the better
fake time is not an intrinsic part of native client

> On Tue, Mar 11, 2014 at 6:34 PM,  <dvyukov@google.com> wrote:
>>
>> https://codereview.appspot.com/73110043/diff/40001/src/pkg/runtime/proc.c
>> File src/pkg/runtime/proc.c (right):
>>
>>
https://codereview.appspot.com/73110043/diff/40001/src/pkg/runtime/proc.c#new...
>> src/pkg/runtime/proc.c:2510: if((gp = runtime·timejump()) != nil) {
>> this scares me
>>
>> here is a lock order inversion between schedlock and timerlock,
>> currently we take timerlock->schedlock, and this adds
>> schedlock->timerlock
>>
>> it's possible that when timejump is executed, no other code is running
>> but it's unclear to me whether it's the case with e.g. external threads
>> (extram)
>>
>> there is also a possibility that timejump does not work with external
>> threads at all (never executed)
>>
>> how general is this functionality intended to be?
>>
>> https://codereview.appspot.com/73110043/

[dave_cheney.net, 2014-03-11 09:34:00]

*** Abandoned ***