FAQ
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@googlegroups.com,

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


Description:
time: Sleep does better job then runtime.Gosched in TestAfterStress

for slow windows-386 builder

Please review this at https://codereview.appspot.com/7128053/

Affected files:
M src/pkg/time/sleep_test.go


Index: src/pkg/time/sleep_test.go
===================================================================
--- a/src/pkg/time/sleep_test.go
+++ b/src/pkg/time/sleep_test.go
@@ -56,7 +56,7 @@
runtime.GC()
// Need to yield, because otherwise
// the main goroutine will never set the stop flag.
- runtime.Gosched()
+ Sleep(Nanosecond)
}
}()
c := Tick(1)

Search Discussions

  • Dave Cheney at Jan 18, 2013 at 1:18 am
    Nice fix, but I wonder if it is masking a deeper issue.
    On 18 Jan 2013 12:16, wrote:

    Reviewers: golang-dev_googlegroups.com,

    Message:
    Hello golang-dev@googlegroups.com,

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


    Description:
    time: Sleep does better job then runtime.Gosched in TestAfterStress

    for slow windows-386 builder

    Please review this at https://codereview.appspot.**com/7128053/<https://codereview.appspot.com/7128053/>

    Affected files:
    M src/pkg/time/sleep_test.go


    Index: src/pkg/time/sleep_test.go
    ==============================**==============================**=======
    --- a/src/pkg/time/sleep_test.go
    +++ b/src/pkg/time/sleep_test.go
    @@ -56,7 +56,7 @@
    runtime.GC()
    // Need to yield, because otherwise
    // the main goroutine will never set the stop flag.
    - runtime.Gosched()
    + Sleep(Nanosecond)
    }
    }()
    c := Tick(1)

  • Alex Brainman at Jan 18, 2013 at 1:19 am
    This change is, probably, not enough to fix windows-386. It also
    struggles with new TestReset. Perhaps, it is possible to make TestReset
    work by fiddling with timeout values. But, I think, this pc is running
    our of puff in general. It was not fast to start with, but we keep
    adding new tests.

    Alex

    https://codereview.appspot.com/7128053/
  • Alex Brainman at Jan 18, 2013 at 1:20 am

    On 2013/01/18 01:18:32, dfc wrote:
    Nice fix, but I wonder if it is masking a deeper issue.
    Sure thing. Explain yourself, please.

    Alex

    https://codereview.appspot.com/7128053/
  • Russ Cox at Jan 18, 2013 at 3:24 am
    LGTM

    The problem is that the timer thread is waiting to run (the timer has
    gone off but the OS hasn't run the thread in response yet) and so we
    don't see a goroutine waiting, and instead we just keep spinning doing
    our own work. It's hard for the timer thread to compete with that, and
    apparently Windows is okay with letting the one thread run away. It's
    not a goroutine scheduler issue.

    // Yield so that the OS can wake up the timer thread,
    // so that it can generate channel sends for the main goroutine,
    // which will eventually set stop = 1 for us.
    Sleep(1*Nanosecond)
  • Alex Brainman at Jan 18, 2013 at 4:31 am
    *** Submitted as
    https://code.google.com/p/go/source/detail?r=69a06608d3b1 ***

    time: Sleep does better job then runtime.Gosched in TestAfterStress

    for slow windows-386 builder

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


    https://codereview.appspot.com/7128053/
  • Ingo Oeser at Jan 18, 2013 at 10:16 pm
    I know, I'm late in the game.

    Maybe the culprit is that the OS and Go scheduler are not aware about the
    atomics.
    What about giving the schedulers sth. to sleep/wake on?

    So I would suggest sth. like

    func TestAfterStress(t *testing.T) {
    stop := make(chan bool)
    go func() {
    select {
    case _, _ = <- stop:
    }
    }()
    c := Tick(1)
    for i := 0; i < 100; i++ {
    <-c
    }
    close(stop)
    }
  • Alex Brainman at Jan 18, 2013 at 11:29 pm

    On 2013/01/18 22:16:40, ioe wrote:

    Maybe the culprit is that the OS and Go scheduler are not aware about the
    atomics.
    What about giving the schedulers sth. to sleep/wake on?

    I am not 100% clear what is been tested here in the first place. So it
    is hard for me to comment on your proposed approach.

    Alex

    https://codereview.appspot.com/7128053/
  • Russ Cox at Jan 22, 2013 at 6:32 pm
    I think what's there now is fine. Let's do more important things.

    Russ

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedJan 18, '13 at 1:16a
activeJan 22, '13 at 6:32p
posts9
users4
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase