Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1150)

Issue 75130045: code review 75130045: doc: allow buffered channel as semaphore without initia... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 8 months ago by rsc
Modified:
3 years, 8 months ago
Reviewers:
r, kortschak, iant, dvyukov
CC:
golang-codereviews, r, Dominik Honnef, dvyukov, kortschak, iant, 0xjnml
Visibility:
Public.

Description

doc: allow buffered channel as semaphore without initialization This rule not existing has been the source of many discussions on golang-dev and on issues. We have stated publicly that it is true, but we have never written it down. Write it down. Fixes issue 6242.

Patch Set 1 #

Patch Set 2 : diff -r 35d1f708a0d2 https://code.google.com/p/go/ #

Patch Set 3 : diff -r 35d1f708a0d2 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 017d82d5ef30 https://code.google.com/p/go/ #

Total comments: 3

Patch Set 5 : diff -r e2fb4c83d69d https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -24 lines) Patch
M doc/effective_go.html View 1 2 3 4 5 chunks +16 lines, -24 lines 0 comments Download
M doc/go_mem.html View 1 1 chunk +35 lines, -0 lines 0 comments Download

Messages

Total messages: 23
rsc
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
3 years, 8 months ago (2014-03-13 03:05:02 UTC) #1
rsc
R=r but i want dvyukov's review too
3 years, 8 months ago (2014-03-13 03:29:37 UTC) #2
r
LGTM
3 years, 8 months ago (2014-03-13 03:32:03 UTC) #3
r
(by the way, Effective Go does say this, informally, so it is written down.)
3 years, 8 months ago (2014-03-13 03:32:38 UTC) #4
Dominik Honnef
On 2014/03/13 03:32:38, r wrote: > (by the way, Effective Go does say this, informally, ...
3 years, 8 months ago (2014-03-13 03:34:13 UTC) #5
r
NOT LGTM I've changed my mind. This makes things more confusing not less. Compare and ...
3 years, 8 months ago (2014-03-13 05:02:49 UTC) #6
r
OK, now I am once again confused. It's not the first time and it won't ...
3 years, 8 months ago (2014-03-13 05:03:17 UTC) #7
dvyukov
I find this "inverted" semaphore a bit confusing. Probably I would not add the similar ...
3 years, 8 months ago (2014-03-13 09:07:56 UTC) #8
dvyukov
On 2014/03/13 09:07:56, dvyukov wrote: > I find this "inverted" semaphore a bit confusing. Probably ...
3 years, 8 months ago (2014-03-13 09:09:36 UTC) #9
kortschak
That is due to the fact that Effective Go was changed in 1a329995118c because of ...
3 years, 8 months ago (2014-03-13 10:54:40 UTC) #10
r
My take: without careful study and reasoning, this example seems to contradict the claims in ...
3 years, 8 months ago (2014-03-13 11:18:20 UTC) #11
dvyukov
On Thu, Mar 13, 2014 at 3:17 PM, Rob Pike <r@golang.org> wrote: > My take: ...
3 years, 8 months ago (2014-03-13 11:52:48 UTC) #12
rsc
It contradicts Effective Go because you changed effective Go under duress last March. https://codereview.appspot.com/7698045/diff/14001/doc/effective_go.html
3 years, 8 months ago (2014-03-13 13:31:28 UTC) #13
rsc
I have added effective_go.html to this CL. I replaced the Go 1.1 example with the ...
3 years, 8 months ago (2014-03-13 14:20:41 UTC) #14
rsc
My view of the history follows. Certainly Rob and I have always expected the counting ...
3 years, 8 months ago (2014-03-13 15:15:07 UTC) #15
r
LGTM but wait for dvyukov The story is that the memory model now supports the ...
3 years, 8 months ago (2014-03-13 20:50:54 UTC) #16
rsc
Dmitriy, I had a hard time understanding your responses. Do you agree that the CL ...
3 years, 8 months ago (2014-03-13 20:51:32 UTC) #17
kortschak
LGTM Thank you for this.
3 years, 8 months ago (2014-03-13 20:59:48 UTC) #18
iant
LGTM https://codereview.appspot.com/75130045/diff/60001/doc/effective_go.html File doc/effective_go.html (right): https://codereview.appspot.com/75130045/diff/60001/doc/effective_go.html#newcode2951 doc/effective_go.html:2951: var sem = make(chan int, MaxOutstanding) Since the ...
3 years, 8 months ago (2014-03-13 22:03:09 UTC) #19
dvyukov
On 2014/03/13 20:51:32, rsc wrote: > Dmitriy, I had a hard time understanding your responses. ...
3 years, 8 months ago (2014-03-14 05:59:53 UTC) #20
dvyukov
On 2014/03/13 15:15:07, rsc wrote: > My view of the history follows. > > Certainly ...
3 years, 8 months ago (2014-03-14 06:10:15 UTC) #21
0xjnml
https://codereview.appspot.com/75130045/diff/60001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/75130045/diff/60001/doc/go_mem.html#newcode303 doc/go_mem.html:303: limit <- 1 I think the send should be ...
3 years, 8 months ago (2014-03-20 10:34:58 UTC) #22
rsc
3 years, 8 months ago (2014-03-24 23:11:26 UTC) #23
*** Submitted as https://code.google.com/p/go/source/detail?r=8e4393a5626f ***

doc: allow buffered channel as semaphore without initialization

This rule not existing has been the source of many discussions
on golang-dev and on issues. We have stated publicly that it is
true, but we have never written it down. Write it down.

Fixes issue 6242.

LGTM=r, dan.kortschak, iant, dvyukov
R=golang-codereviews, r, dominik.honnef, dvyukov, dan.kortschak, iant, 0xjnml
CC=golang-codereviews
https://codereview.appspot.com/75130045

https://codereview.appspot.com/75130045/diff/60001/doc/go_mem.html
File doc/go_mem.html (right):

https://codereview.appspot.com/75130045/diff/60001/doc/go_mem.html#newcode303
doc/go_mem.html:303: limit <- 1
On 2014/03/20 10:34:58, 0xjnml wrote:
> I think the send should be before the above go statement. I see that the
> mechanism works in the current version. It's only not scaling well for big
> amount of work - a lot of (memory consuming) goroutines are created when only
> some of them can make progress. Moving the send before the go statement avoids
> that.

thanks but no thanks. that's getting to be a distraction from the main point.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld c59f3c4