Issue 101980047: code review 101980047: doc: fix happens-before rules for buffered channels (Closed)
Descriptiondoc: fix happens-before rules for buffered channels
The current wording is reversed in 2 places.
Not sure how it got 4 LGTMs (mine was there as well).
Update issue 6242.
Patch Set 1 #Patch Set 2 : diff -r 7816cf77a4e5 https://dvyukov%40google.com@code.google.com/p/go/ #Patch Set 3 : diff -r 7816cf77a4e5 https://dvyukov%40google.com@code.google.com/p/go/ #
Total comments: 2
Patch Set 4 : diff -r fd0610efce5d https://dvyukov%40google.com@code.google.com/p/go/ #
Total comments: 10
Patch Set 5 : diff -r ccf7893cc2f0 https://dvyukov%40google.com@code.google.com/p/go/ #Patch Set 6 : diff -r ccf7893cc2f0 https://dvyukov%40google.com@code.google.com/p/go/ #MessagesTotal messages: 16
Hello golang-codereviews@googlegroups.com (cc: r@golang.org, rsc@golang.org), I'd like you to review this change to https://dvyukov%40google.com@code.google.com/p/go/
Sign in to reply to this message.
https://codereview.appspot.com/101980047/diff/40001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/101980047/diff/40001/doc/go_mem.html#newcode278 doc/go_mem.html:278: The <i>k</i>th receive on a channel with capacity <i>C</i> happens before the <i>k</i>+<i>C</i>th send from that channel completes. It puzzles me that both the old and new version seems correct to me. Not sure if equivalent, though. Need a cofee.
Sign in to reply to this message.
LGTM (with trepidation and apologies for the previous). https://codereview.appspot.com/101980047/diff/40001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/101980047/diff/40001/doc/go_mem.html#newcode278 doc/go_mem.html:278: The <i>k</i>th receive on a channel with capacity <i>C</i> happens before the <i>k</i>+<i>C</i>th send from that channel completes. The previous wording is redundant since the sentence at L193 covers that - and worse it didn't cover what issue 6242 was asking for. On 2014/05/31 09:01:57, 0xjnml wrote: > It puzzles me that both the old and new version seems correct to me. Not sure if > equivalent, though. Need a cofee.
Sign in to reply to this message.
bumped doc version date in patch set 4
Sign in to reply to this message.
LGTM i think. the saga continues. wait for rsc.
Sign in to reply to this message.
On 2014/05/31 16:32:37, r wrote: > LGTM i think. the saga continues. wait for rsc. Russ, does it look correct to you?
Sign in to reply to this message.
https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode284 doc/go_mem.html:284: the number of free items in the channel buffer corresponds to the semaphore count, Maybe I do not know the semaphore terminology, but it seems to me that the 'semaphore count' is the number of acquires without matching releases, in which case the original text is correct and this text is wrong.
Sign in to reply to this message.
https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode284 doc/go_mem.html:284: the number of free items in the channel buffer corresponds to the semaphore count, On 2014/06/03 15:35:23, rsc wrote: > Maybe I do not know the semaphore terminology, but it seems to me that the > 'semaphore count' is the number of acquires without matching releases, in which > case the original text is correct and this text is wrong. Semaphore count is number of releases w/o matching acquires. See man sem_post: http://www.gsp.com/cgi-bin/man.cgi?section=3&topic=sem_post The sem_post function increments (unlocks) the semaphore pointed to by sem. And WinAPI CreateSemaphore: http://msdn.microsoft.com/en-us/library/windows/desktop/ms682438(v=vs.85).aspx The state of a semaphore is signaled when its count is greater than zero and nonsignaled when it is zero. The count is decreased by one whenever a wait function releases a thread that was waiting for the semaphore. The count is increased by a specified amount by calling the ReleaseSemaphore function.
Sign in to reply to this message.
For an idea I have been told repeatedly is so much easier to understand that locks, semaphores generate an inordinate amount of confusion. -rob
Sign in to reply to this message.
https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode284 doc/go_mem.html:284: the number of free items in the channel buffer corresponds to the semaphore count, If I have to read manuals to understand this sentence, something is wrong. Let's avoid the term entirely. It allows a counting semaphore to be modeled by a buffered channel: the number of items in the channel corresponds to the number of active uses, the capacity of the channel corresponds to the maximum number of simultaneous uses, sending...
Sign in to reply to this message.
https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode278 doc/go_mem.html:278: The <i>k</i>th receive on a channel with capacity <i>C</i> happens before the <i>k</i>+<i>C</i>th send from that channel completes. It took me a while but I believe this line is correct and apropos. It is the generalization of the previous rule (A receive from an unbuffered channel happens before the send on that channel completes.)
Sign in to reply to this message.
https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode278 doc/go_mem.html:278: The <i>k</i>th receive on a channel with capacity <i>C</i> happens before the <i>k</i>+<i>C</i>th send from that channel completes. On 2014/06/03 16:17:45, rsc wrote: > It took me a while but I believe this line is correct and apropos. > It is the generalization of the previous rule > (A receive from an unbuffered channel happens before the send on that channel > completes.) do you mean the old line or the new line? https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode284 doc/go_mem.html:284: the number of free items in the channel buffer corresponds to the semaphore count, On 2014/06/03 16:15:43, rsc wrote: > If I have to read manuals to understand this sentence, something is wrong. Let's > avoid the term entirely. > > It allows a counting semaphore to be modeled by a buffered channel: > the number of items in the channel corresponds to the number of active uses, > the capacity of the channel corresponds to the maximum number of simultaneous > uses, > sending... active uses of what? https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode288 doc/go_mem.html:288: This is a common idiom for rate-limiting work. Doesn't this sentence need to be rephrased as well? The following code does not limit rate of work, it limits concurrency level.
Sign in to reply to this message.
https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html File doc/go_mem.html (right): https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode278 doc/go_mem.html:278: The <i>k</i>th receive on a channel with capacity <i>C</i> happens before the <i>k</i>+<i>C</i>th send from that channel completes. On 2014/06/04 08:54:31, dvyukov wrote: > On 2014/06/03 16:17:45, rsc wrote: > > It took me a while but I believe this line is correct and apropos. > > It is the generalization of the previous rule > > (A receive from an unbuffered channel happens before the send on that channel > > completes.) > > do you mean the old line or the new line? I mean the new line is correct. https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode284 doc/go_mem.html:284: the number of free items in the channel buffer corresponds to the semaphore count, On 2014/06/04 08:54:31, dvyukov wrote: > On 2014/06/03 16:15:43, rsc wrote: > > If I have to read manuals to understand this sentence, something is wrong. > Let's > > avoid the term entirely. > > > > It allows a counting semaphore to be modeled by a buffered channel: > > the number of items in the channel corresponds to the number of active uses, > > the capacity of the channel corresponds to the maximum number of simultaneous > > uses, > > sending... > > active uses of what? of whatever the semaphore protects. If you don't like my wording, propose something new. But we need to avoid reference to the integer value of the semaphore. https://codereview.appspot.com/101980047/diff/60001/doc/go_mem.html#newcode288 doc/go_mem.html:288: This is a common idiom for rate-limiting work. sure. say 'for limiting concurrency.'
Sign in to reply to this message.
Done PTAL
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=12c9a9ff50d8 *** doc: fix happens-before rules for buffered channels The current wording is reversed in 2 places. Not sure how it got 4 LGTMs (mine was there as well). Update issue 6242. LGTM=dan.kortschak, r, rsc R=golang-codereviews, 0xjnml, dan.kortschak, r, rsc CC=golang-codereviews https://codereview.appspot.com/101980047
Sign in to reply to this message.
|
