Ineffective breaks in heap_manager.go

[egonelbre]

I was looking through the codebase and when running staticcheck it lists these issues:

heap_manager.go:81:6: ineffective break statement. Did you mean to break out of the outer loop? (SA4011)
heap_manager.go:91:6: ineffective break statement. Did you mean to break out of the outer loop? (SA4011)

• mpb/heap_manager.go

  Line 80 in d8400e9

  case <-data.drop:

• mpb/heap_manager.go

  Line 90 in d8400e9

  case <-data.drop:

I'm not sure whether you want to return or just exit the bHeap handling.

---

Other things I noticed that aren't errors per se, but looked unusual to me:

mpb/decor/eta.go

Line 203 in d8400e9

return ((remaining / time.Second) * time.Second).String()

time.Duration has Truncate func: https://pkg.go.dev/time#Duration.Truncate

https://github.com/vbauerster/mpb/blob/d8400e981502dafd3884e25d826a01ae659a74c2/decor/pool.go

using a var buffer [64]byte should get you most of the way and avoid scenarios where you accidentally pool large []byte values for a long time.

[vbauerster]

I was looking through the codebase and when running staticcheck it lists these issues:

Yep, staticcheck's suggestion is right, despite current behavior doesn't lead to any bugs.

Other things I noticed that aren't errors per se, but looked unusual to me:

Thankfully I left a comment strip off nanoseconds. I agree that Truncate func is better option here, thanks for suggestion.

using a var buffer [64]byte should get you most of the way and avoid scenarios where you accidentally pool large []byte values for a long time.

It's used to feed strconv.AppendFloat with dst []byte param. Did you mean to increase cap to 64 like b := make([]byte, 0, 64) or using array [64]byte instead of slice? If latter then how can it help/improve?

[egonelbre]

I meant the var buffer [64]byte version. Take a look at https://go.dev/play/p/0ig2yQgA3gg.

Benchmark results:

name              time/op
Append_Pooled-32  214ns ± 5%
Append_Stack-32   202ns ± 1%
Append_Alloc-32   201ns ± 0%

Also, avoiding the pool is also going to less code and easier to reason about.

While reviewing the code, I realized that that it's always putting back the small slice, so there's no problem with holding on to a potentially large slices.

[vbauerster]

Addressed in v8.4.0.