Added call to runtime.Gosched() to goroutines #104

Open
wants to merge 2 commits into
from

2 participants

@ysagal

With an explanation for why you may need it in order to observe interleaved execution. Fixes #103

Not sure if this is clear or just complicates the example, but I imagine some people are confused when they run the sample code and don't see the expected outcome.

@schmichael schmichael commented on the diff Jun 16, 2015
examples/goroutines/goroutines.go
func f(from string) {
for i := 0; i < 3; i++ {
fmt.Println(from, ":", i)
+
+ // `runtime.GoSched()` transfers execution to the next
+ // goroutine. Since goroutines are multiplexed across
+ // system threads, if there's only one thread it won't
+ // yield to another goroutine unless asked explicitly
+ // as with the call as below or during a system call.
+ // Note that you can control the number of system
+ // threads with the `GOMAXPROCS` environment variable.
+ runtime.Gosched()
@schmichael
schmichael added a line comment Jun 16, 2015

This comment is wrong and the Gosched call is misleading. Explicitly calling Gosched is rare and probably has no place in examples for newcomers to Go.

Even if fmt.Println didn't cause a syscall (which it does), as of Go 1.2 calling functions is sufficient to cause goroutine preemption: https://golang.org/doc/go1.2#preemption

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ysagal

Thanks for reviewing! The current example, however, does not seem to yield from f() unless requested explicitly and doesn't run in the playground either. What would be the appropriate fix so people can observe the output in the example for themselves?

@schmichael

@ysagal It doesn't work in the playground because the playground doesn't support blocking on stdin. Using WaitGroups or chans would be the idiomatic Go solution, but probably too complicated for this initial trivial goroutine example.

Using time.Sleep(time.Second) isn't idiomatic, but accomplishes the same thing as blocking on stdin and works in the playground.

Honestly I'm not sure whether it's better to be idiomatic-but-complex (waitgroup) or non-idiomatic-but-simple (time.sleep). But Gosched is neither idiomatic nor simple.

@ysagal

Thanks for the explanation. I guess I'm still not sure why preemption doesn't work even with time.Sleep(): http://play.golang.org/p/wfXnyDgmSi

Chans are covered in the following examples, so I think it would be fine to keep this one really simple (even if non-idiomatic), as long as readers are able to reproduce the output either in the playground or running locally, which isn't the case at the moment.

@schmichael

You're demonstrating concurrency just fine in this example. Preemption is a runtime feature which varies between versions and probably implementations, so attempting to demonstrate it is going to be inherently awkward and fragile.

I would personally just stay away from it until you have an example where multiple goroutines communicate over chans -- then the interleaving of concurrent processes becomes obvious as goroutines are scheduled as their chans wakeup.

However if you want to try to hack an example of preemption in this early time.Sleep is effectively identical to runtime.Gosched -- neither is something to encourage anyone to do in "real" code and should be marked clearly as such. Perhaps the clearest way I've seen this done is something like:

func DoWork() {
    // Some resource intensive work takes place here
    time.Sleep(time.Second)
}

Where it's clear to readers that you're just faking a placeholder for say a database lookup or request handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment