statsdaemon icon indicating copy to clipboard operation
statsdaemon copied to clipboard

Parallelize submission

Open JensRantil opened this issue 10 years ago • 5 comments

Make the flush operation execute in parallel.

These optimizations have been mentioned in #23.

Additional comments:

  • I'm not sure they are worth incorporating as they might do more harm than good. That said, maybe they can foster a good discussion about possible speed optimizations. I don't know.
  • I know good Go practices are to avoid the sync package and only use channels. In this case, I thought sync.WaitGroup felt like they were making the code more understandable. I might be wrong.

JensRantil avatar Nov 12 '13 14:11 JensRantil

Fixed merge conflicts. Feel free to revisit.

JensRantil avatar May 27 '14 10:05 JensRantil

Thanks @JensRantil for this contributions. Can you share some benchmark deltas for this changeset?

jehiah avatar May 27 '14 20:05 jehiah

Can you share some benchmark deltas for this changeset?

Feels like I made this pull request an eternity ago. Anyway, here are some benchmarks:


➜  statsdaemon git:(master) go test -bench=.
PASS
BenchmarkManyDifferentSensors          1    6731116058 ns/op
BenchmarkOneBigTimer           1    6180231467 ns/op
BenchmarkLotsOfTimers          1    6790958617 ns/op
ok      github.com/bitly/statsdaemon    35.471s
➜  statsdaemon git:(master) git checkout parallelize-submission
➜  statsdaemon git:(parallelize-submission) go test -bench=.
PASS
BenchmarkManyDifferentSensors          1    3241844699 ns/op
BenchmarkOneBigTimer           1    3378769659 ns/op
BenchmarkLotsOfTimers          1    6503543688 ns/op
ok      github.com/bitly/statsdaemon    30.082s

So, master is faster or equal in speed.

Oh, also, the tests seem to pass despite Travis failing this:

➜  statsdaemon git:(parallelize-submission) go test
PASS
ok      github.com/bitly/statsdaemon    0.018s

JensRantil avatar Jun 01 '14 18:06 JensRantil

travis test failure was "index out of range" when indexing into position 0 of the slice t:

func processTimer(buffer *bytes.Buffer, now int64, pctls Percentiles, u string, t Uint64Slice) {
    sort.Sort(t)
    min := t[0]

Also, it failed while testing with go1.0.3 - is it possible that some difference between the version of go you use and go1.0.3 means t could be an empty slice here? Or maybe a race condition when you enable multiple GOMAXPROCS?

ploxiln avatar Jun 06 '14 23:06 ploxiln

I tried to recreate this. So far, nothing:

➜  statsdaemon git:(parallelize-submission) ✗ go version
go version go1.2.2 darwin/amd64
➜  statsdaemon git:(parallelize-submission) ✗ go test
PASS
ok      github.com/bitly/statsdaemon    0.009s
➜  statsdaemon git:(parallelize-submission) ✗ GOMAXPROCS=8 go test
PASS
ok      github.com/bitly/statsdaemon    0.010s

I am running Go 1.1 branch. Do you have a Go 1.0 installation at hand perhaps? Can you recreate this?

JensRantil avatar Jun 15 '14 15:06 JensRantil