Improve memory of producer retry queue #396
Conversation
The old version just used a simple slice as a queue, using append and slice[1:].
This had correct (amortized O(1)) running time, but did not behave well with the
garbage collector. In particular:
- elements removed from the slice were still pointed to by the underlying array,
preventing them from being collected until the slice was resized
- the slice would only be resized when its current capacity was "full", even if
it had a substantial amount of free space it was wasting
Switch instead to my queue implementation (which I wrote a while ago to solve
these exact problems somewhere else). It uses a ringbuffer that is guaranteed to
stay with a factor of 2 size of the actual number of messages stored, it
generates a lot less garbage when the input and output rates are about the same,
and it nils removed elements in the underlying array so the garbage collector
can pick them up at its leisure.
The only possible concern is that it uses `interface{}` but the usage is so
restrained that type-safety is trivial and the performance will not be noticable
given everything else the producer has to do.
|
How this this compare to container/heap? |
|
A queue is not a heap... and |
|
I find it interesting go doesn't have an efficient queue in its standard library. I guess it just part of the general lack of container types due to the lack of generics. Oh well. Anyway, this is 👍 for me, if the queue implementation is properly tested. |
|
The queue is pretty simple: https://github.com/eapache/queue/blob/master/queue.go and has some tests: https://github.com/eapache/queue/blob/master/queue_test.go |
|
There is http://golang.org/pkg/container/list/ that we could use, but it allocates a new struct for each element which makes it somewhat less efficient (in terms of both memory and gc) |
|
If we'd rather stick with the stdlib though, I can't imagine the overhead would be too terrible (again, at least compared to all the other things that the producer has to do). |
|
Given that it's only 89 lines of go, using this lib is OK I think 👍 |
|
This code lgtm, I'm blindly trusting your queue implementation though. |
|
Hmm, now I am tempted to switch to container/list just on the principle of "write less code". |
|
Eh, the queue implementation is also used (and thus pretty heavily tested implicitly) in https://github.com/eapache/channels and it's not like it's going to change much. |
Improve memory of producer retry queue
The old version just used a simple slice as a queue, using append and slice[1:].
This had correct (amortized O(1)) running time, but did not behave well with the
garbage collector. In particular:
preventing them from being collected until the array was resized
it had a substantial amount of free space it was wasting
Switch instead to my queue implementation (which I wrote a while ago to solve
these exact problems somewhere else). It uses a ringbuffer that is guaranteed to
stay with a factor of 2 size of the actual number of messages stored, it
generates a lot less garbage when the input and output rates are about the same,
and it nils removed elements in the underlying array so the garbage collector
can pick them up at its leisure.
The only possible concern is that it uses
interface{}but the usage is sorestrained that type-safety is trivial and the performance will not be noticable
given everything else the producer has to do.
@Shopify/kafka