Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Effective Go should use sync.WaitGroup, not dummy channel, for Parallelization example #13175

Open
ArchRobison opened this issue Nov 6, 2015 · 8 comments
Labels
Milestone

Comments

@ArchRobison
Copy link

@ArchRobison ArchRobison commented Nov 6, 2015

This is a documentation issue.

The section in Effective Go on parallelization shows how to use a channel to count goroutine completions. This seems like bad advice for Go learners, since sync.WaitGroup is a more concise alternative that is expressly designed for this idiom.

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Nov 6, 2015
@ianlancetaylor ianlancetaylor changed the title Effective Go should use sync.WaitGroup, not dummy channel, for Parallelization example doc: Effective Go should use sync.WaitGroup, not dummy channel, for Parallelization example Nov 6, 2015
@cznic
Copy link
Contributor

@cznic cznic commented Nov 6, 2015

IMHO the discussed example is way better as it is now than complicating it with sync.WaitGroup.

  • The functionality of sync.WaitGroup would have to be explained - in detail.
  • sync.WaitGroup has issues the simple channel solution does not suffer from.
@ArchRobison
Copy link
Author

@ArchRobison ArchRobison commented Nov 8, 2015

The full power of sync.WaitGroup does not have to be explained, just the restricted idiom of using it to count and wait on a known number of completion signals.

What are the issues that using sync.WaitGroup would have that the channel solution does not?

@minux
Copy link
Member

@minux minux commented Nov 8, 2015

@stemar94
Copy link

@stemar94 stemar94 commented Nov 8, 2015

For this kind of synchronization(if using a channel), one would normally choose a chan struct{}, to make clear that one is only interested in the synchronization events and not the content transported through the channel. At least this could be changed.

@cznic
Copy link
Contributor

@cznic cznic commented Nov 8, 2015

It's rather easy to understand what's going on when sending 1 to a chan int. Some (many?) newbies may not have (yet) any idea what struct{} and/or struct{}{} even means.

Why make stuff less comprehensible? The discussed example is just fine as it is now. It's goal is to explain/illustrate some principle, not to teach how to write the most effective/performant/shortest/you-name-it code possible.

@adg
Copy link
Contributor

@adg adg commented Nov 9, 2015

@ArchRobison
Copy link
Author

@ArchRobison ArchRobison commented Nov 9, 2015

The example ought to at least close with a mention of WaitGroup. Otherwise newcomers (like myself) are left with the impression that Go's support for task-based parallelism is circuitous.

@adg
Copy link
Contributor

@adg adg commented Nov 9, 2015

I think that's reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.