Skip to content

Commit

Permalink
taskgroup: drop *Group from the signature of start functions (#7)
Browse files Browse the repository at this point in the history
While it is mildly convenient in a few cases to chain construction of the group
with starting a goroutine, it has turned out not to be worthwhile in practice.
Apart from the tests (which are hereby updated not to do that anymore), a
search of GitHub suggests nobody uses this -- even in code I wrote.
  • Loading branch information
creachadair authored Oct 5, 2024
1 parent 3dbd8f6 commit 787f319
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 28 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ A task is expressed as a `func() error`, and is added to a group using the `Go`
method:

```go
g := taskgroup.New(nil).Go(myTask)
var g taskgroup.Group
g.Go(myTask)
```

Any number of tasks may be added, and it is safe to do so from multiple
Expand Down
36 changes: 18 additions & 18 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,24 +187,24 @@ func ExampleCollector_Report() {
}
c := taskgroup.Collect(func(z val) { fmt.Println(z.who, z.v) })

err := taskgroup.New(nil).
// The Report method passes its argument a function to report multiple
// values to the collector.
Go(c.Report(func(report func(v val)) error {
for i := range 3 {
report(val{"even", 2 * i})
}
return nil
})).
// Multiple reporters are fine.
Go(c.Report(func(report func(v val)) error {
for i := range 3 {
report(val{"odd", 2*i + 1})
}
// An error from a reporter is propagated like any other task error.
return errors.New("no bueno")
})).
Wait()
g := taskgroup.New(nil)
// The Report method passes its argument a function to report multiple
// values to the collector.
g.Go(c.Report(func(report func(v val)) error {
for i := range 3 {
report(val{"even", 2 * i})
}
return nil
}))
// Multiple reporters are fine.
g.Go(c.Report(func(report func(v val)) error {
for i := range 3 {
report(val{"odd", 2*i + 1})
}
// An error from a reporter is propagated like any other task error.
return errors.New("no bueno")
}))
err := g.Wait()
if err == nil || err.Error() != "no bueno" {
log.Fatalf("Unexpected error: %v", err)
}
Expand Down
7 changes: 3 additions & 4 deletions taskgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ func (g *Group) activate() {
// manipulate local data structures without additional locking.
func New(ef ErrorFunc) *Group { return &Group{onError: ef} }

// Go runs task in a new goroutine in g, and returns g to permit chaining.
func (g *Group) Go(task Task) *Group {
// Go runs task in a new goroutine in g.
func (g *Group) Go(task Task) {
g.wg.Add(1)
if g.active.Load() == 0 {
g.activate()
Expand All @@ -67,14 +67,13 @@ func (g *Group) Go(task Task) *Group {
g.handleError(err)
}
}()
return g
}

// Run runs task in a new goroutine in g, and returns g to permit chaining.
// This is shorthand for:
//
// g.Go(taskgroup.NoError(task))
func (g *Group) Run(task func()) *Group { return g.Go(NoError(task)) }
func (g *Group) Run(task func()) { g.Go(NoError(task)) }

func (g *Group) handleError(err error) {
g.μ.Lock()
Expand Down
15 changes: 10 additions & 5 deletions taskgroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func TestBasic(t *testing.T) {
t.Logf("Group value is %d bytes", reflect.TypeOf((*taskgroup.Group)(nil)).Elem().Size())

// Verify that the group works at all.
g := taskgroup.New(nil).Go(busyWork(25, nil))
g := taskgroup.New(nil)
g.Go(busyWork(25, nil))
if err := g.Wait(); err != nil {
t.Errorf("Unexpected task error: %v", err)
}
Expand Down Expand Up @@ -61,7 +62,8 @@ func TestErrorPropagation(t *testing.T) {
defer leaktest.Check(t)()

var errBogus = errors.New("bogus")
g := taskgroup.New(nil).Go(func() error { return errBogus })
var g taskgroup.Group
g.Go(func() error { return errBogus })
if err := g.Wait(); err != errBogus {
t.Errorf("Wait: got error %v, wanted %v", err, errBogus)
}
Expand Down Expand Up @@ -152,7 +154,8 @@ func TestCapacity(t *testing.T) {
func TestRegression(t *testing.T) {
t.Run("WaitRace", func(t *testing.T) {
ready := make(chan struct{})
g := taskgroup.New(nil).Go(func() error {
g := taskgroup.New(nil)
g.Go(func() error {
<-ready
return nil
})
Expand Down Expand Up @@ -205,7 +208,8 @@ func TestSingleTask(t *testing.T) {
return <-release
})

g := taskgroup.New(nil).Run(func() {
g := taskgroup.New(nil)
g.Run(func() {
if err := s.Wait(); err != sentinel {
t.Errorf("Background Wait: got %v, want %v", err, sentinel)
}
Expand Down Expand Up @@ -309,7 +313,8 @@ func TestCollector_Report(t *testing.T) {
var sum int
c := taskgroup.Collect(func(v int) { sum += v })

g := taskgroup.New(nil).Go(c.Report(func(report func(v int)) error {
var g taskgroup.Group
g.Go(c.Report(func(report func(v int)) error {
for _, v := range rand.Perm(10) {
report(v)
}
Expand Down

0 comments on commit 787f319

Please sign in to comment.