zmq4 icon indicating copy to clipboard operation
zmq4 copied to clipboard

consider using a pool of zmq4.Msg to reduce alloc pressure

Open sbinet opened this issue 6 years ago • 2 comments

right now, the Socket interface reads:

type Socket interface {
	// Send puts the message on the outbound send queue.
	// Send blocks until the message can be queued or the send deadline expires.
	Send(msg Msg) error

	// Recv receives a complete message.
	Recv() (Msg, error)

	...
}

internally, each time somebody calls sck.Recv() a new zmq4.Msg is created, anew.

we could perhaps consider going with:

type Socket interface {
	Recv(*Msg) error
}

so the Socket implementation wouldn't have to allocate (and let the user optionally use a pool).

alternatively:

  • leave the API of Socket as is,
  • use a pool internally (ie: inside the Socket implementation)
  • add a Release method to zmq4.Msg (that calls back to sync.Pool.Put)

sbinet avatar Jun 15 '18 13:06 sbinet

Happy Halloween! And now for the gruesome details.

To make messages reusable the whole API needs to change to pointers. Also the direct access to the .Frames needs to be forbidden, because this and the attached frame data would be managed resources.

I experimented with a little slab allocator for frame data tied to Conn.read where the messages are created. It already shows some reduction in allocations -- given my unscientific and crude measurement.

Actually I'm missing a point to attach the memory management to. I'm old and missing the good old zmqContext. ;) Having a context entity would allow the memory management be effective for all sockets in this context.

Given the asynchronous nature of the receive process a RecvInto(*Msg) seems not possible. The actual reads are done in separate go routines and there is no way to pass the message to the reader who will receive the next message. IMHO the reader should create the message without allocating everything as new and return a single pointer.

The default behaviour of messages should be the same as now. They will be gc'ed when they are not accessible any more.

A new method like Msg.Release() would recycle the internal storage of the message. This would include the frame data (frame = []byte) and the associated container ([]frame). I think keeping whole messages around would waste a lot of memory, because this would keep the frame structure and data regardless of size.

So the public interface for receiving message could be:

type Socket interface {
    Recv() (*Msg, error)
}

As like now Conn.read would create and populate a new message. Instead of allocating every frame anew it would re-use already allocated frames. This works nice for the NullSecurity case.

For the encryption case we're basically half-screwed. The original read of the frame data is the same as for the unencrypted case. But then we need to decrypt the data which results in a unknown size. A simple optimization would be to request a slab size one increment larger as the raw data. If this is not enough fall back to raw byte allocation. After decryption the raw data can already be recycled.

The new message interface needs to be determined during the transition process. A first try could be:

type Message interface {
    // ownership is transferred to the message!
    AppendFrames(frames ...[]byte) error
    // ownership is still at the message!
    Frame(i int) []byte

    // Recycle all internal storage (frames etc.)
    Release()
}

Hoping for a fruitful discussion.

Cheers Guido

guidog avatar Oct 31 '20 18:10 guidog

when I "designed" go-zmq4, I had looked at various sources (rants about 0MQ from the original author, docs from nanomsg, etc...). I don't recall the specifics but it seemed to me the general agreement was: "zmq_context_t was a bad idea".

that said, it does seem like an obvious place were we to store a memory allocator. we could shove it via yet another WithMemAllocator func option.

wrt the AppendFrames ownership transfer, even if there are precedents in Go (e.g.: values transferred over channels are supposed to also transfer ownership), it's not (if I am not mistaken) a very widely used API design. but ok. we could start like that and see how it goes (changing from stealing to copying is less error prone than the other way around)

sbinet avatar Nov 06 '20 15:11 sbinet