Skip to content

Fixed bug with nested Run blocks#1

Open
brodie-roberts-ix wants to merge 1 commit intobenburkert:masterfrom
brodie-roberts-ix:master
Open

Fixed bug with nested Run blocks#1
brodie-roberts-ix wants to merge 1 commit intobenburkert:masterfrom
brodie-roberts-ix:master

Conversation

@brodie-roberts-ix
Copy link
Copy Markdown

@brodie-roberts-ix brodie-roberts-ix commented Jan 11, 2019

I've been using your package for a project I'm working on at work. I found a bug when nesting Run blocks, which can be reproduced like so:

func BenchmarkABC(tb *testing.B) {
	b := pbench.New(tb)
	b.ReportPercentile(0.99)

	b.Run("D", func(b *pbench.B) {
		b.Run("E", func(b *pbench.B) {
			b.RunParallel(func(pb *pbench.PB) {
				for pb.Next() {
					fmt.Print(".")
					time.Sleep(100 * time.Millisecond)
				}
			})
		})
	})
}

This one-line change should fix the issue!

@brodie-roberts-ix
Copy link
Copy Markdown
Author

Upon using this further I've found a second panic case once two such Run blocks are put consecutively. The panic is on line 91 of pbench.go and triggers when Run "F" returns. I can't figure this one out easily, so I'll just have to fall back to only nesting Runs 1 deep.

func BenchmarkABC(tb *testing.B) {
	b := pbench.New(tb)
	b.ReportPercentile(0.99)

	b.Run("D", func(b *pbench.B) {
		b.Run("E", func(b *pbench.B) {
			b.RunParallel(func(pb *pbench.PB) {
				for pb.Next() {
					fmt.Print(".")
					time.Sleep(100 * time.Millisecond)
				}
			})
		})
	})

	b.Run("F", func(b *pbench.B) {
		b.Run("G", func(b *pbench.B) {
			b.RunParallel(func(pb *pbench.PB) {
				for pb.Next() {
					fmt.Print(".")
					time.Sleep(100 * time.Millisecond)
				}
			})
		})
	}) // triggers here
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant