code review 7698045: effective_go.html: fix semaphore example

effective_go.html: fix semaphore example
It didn't work properly according to the Go memory model.
Fixes issue 5023.

[r, 2013-03-11 18:36:23]

Hello golang-dev@googlegroups.com,

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

[dvyukov, 2013-03-11 18:50:39]

https://codereview.appspot.com/7698045/diff/1/src/pkg/debug/macho/file.go
File src/pkg/debug/macho/file.go (right):

https://codereview.appspot.com/7698045/diff/1/src/pkg/debug/macho/file.go#new...
src/pkg/debug/macho/file.go:146: // not have the correct format for an object
file.
Does it belong here?

[dvyukov, 2013-03-11 18:52:13]

https://codereview.appspot.com/7698045/diff/1/doc/effective_go.html
File doc/effective_go.html (right):

https://codereview.appspot.com/7698045/diff/1/doc/effective_go.html#newcode2901
doc/effective_go.html:2901: so during initialization we prime the channel  by
filling it to capacity.
double space

[dvyukov, 2013-03-11 19:02:59]

The change looks good, but I have the following suggestion while we are here:

https://codereview.appspot.com/7698045/diff/1/doc/effective_go.html
File doc/effective_go.html (right):

https://codereview.appspot.com/7698045/diff/1/doc/effective_go.html#newcode2915
doc/effective_go.html:2915: go handle(req)  // Don't wait for handle to finish.
There are 2 ways to implement this pattern. One is this. And the other is:

func Serve(queue chan *Request) {
  sem := make(chan int, MaxOutstanding)
  for i := 0; i < MaxOutstanding; i++ {
    sem <- 1
  }
  for req := range queue {
    <-sem
    go func() {
      process(req)
      sem <- 1
    }
  }
}

The difference is that the current way potentially receives unbounded amount of
requests from the queue and spawns unbounded number of goroutines, it's just
calls to process() that are limited.

I've seen a few instances of the current approach in the group, and I suspect
they slip from this doc. However I do not see any reason to use it instead of
the approach above.
Do we want to change it to not suggest creation of unbounded number of
goroutines?

[r, 2013-03-11 19:32:22]

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

Please take another look.

[adg1, 2013-03-12 01:06:17]

LGTM
On 12 Mar 2013 06:32, <r@golang.org> wrote:

> Hello golang-dev@googlegroups.com, dvyukov@google.com (cc:
> golang-dev@googlegroups.com),
>
> Please take another look.
>
>
>
https://codereview.appspot.**com/7698045/<https://codereview.appspot.com/7698...
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[dvyukov, 2013-03-12 04:10:02]

LGTM

The second version has another serious advantage -- it applies backpressure on
the incoming chan in case of overload. It's a nice feature of bounded channels
-- overload automatically propagates over the graph. The first version happily
consumes everything that comes in.

[r, 2013-03-12 17:53:06]

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

effective_go.html: fix semaphore example
It didn't work properly according to the Go memory model.
Fixes issue 5023.

R=golang-dev, dvyukov, adg
CC=golang-dev
https://codereview.appspot.com/7698045