Support batch allocation of control_t with zmq_msg_init_data

[sigiesec]

We are using zmq_msg_init_data to create messages since we allocate the underlying buffers using a pool to avoid dynamic memory allocation. However, this is defeated by the implementation of zmq_msg_init_data using malloc for its control_t structure.

[bluca]

I had looked at it a year ago or so, and my conclusion was that it just wasn't doable otherwise. Unfortunately I don't remember the details, I'll look again and try to recall.

[sigiesec]

Hm, can't the control_t structure just be made a member of msg_t?

[bluca]

IIRC it didn't fit in 64 bytes

[sigiesec]

Why is it esssential that msg_t_size is 64? What if it were slightly larger?

Another idea: The user could be allowed to provide the storage for content_t, and it could his responsibility to free it in his msg_free_fn. Not completely sure if this would work out.

[bluca]

It's part of the public ABI, and a fundamental one as well so it would break all applications, and all bindings as well. It was a huge pain when we moved from 32 to 64, so it can't be done lightly.

Also I've measured it at the time and it actually decreased performance since the message didn't fit into a cache line anymore.

[sigiesec]

Ok, I see. What about the idea to let the user provide storage for content_t, in an additional zmq_msg_init_* function. Do you think this would be feasible?

[bluca]

Not sure, exposing a very internal detail like that is an additional public ABI - the moment we want to change it, we cannot as it would break applications. It also makes the API more complicated :-/

Especially since it embeds an internal object, the atomic counter - size of that object might vary depending on the platform - for example on older platforms where there's no intrinsics it just uses a mutex. In other cases it's just a counter with some inline assembly instructions.

[sigiesec]

The size of content_t is platform-dependent of course. Already the size of the pointers is platform-dependent. I don't think there is a need for the size to be platform-neutral, the ABI is platform-dependent anyway. To be somewhat safe against changes, the indicated size may be somewhat larger than the current size. For messages that are not very small, this would probably be much less overhead than the extra memory allocation.

Internally, such co-allocation is also done, and it is done for a reason, which is equally relevant for external uses IMO.

ABI-compatibility is also not an issue for certain uses (such as ours), we build with LTCG against libzmq-static anyway.

Would you find it acceptable if I added such a function, of course as DRAFT API? Before un-DRAFTing it, this can be thoroughly reconsidered.

[bluca]

The size of content_t is platform-dependent of course. Already the size of the pointers is platform-dependent. I don't think there is a need for the size to be platform-neutral, the ABI is platform-dependent anyway. To be somewhat safe against changes, the indicated size may be somewhat larger than the current size. For messages that are not very small, this would probably be much less overhead than the extra memory allocation.

Internally, such co-allocation is also done, and it is done for a reason, which is equally relevant for external uses IMO.

ABI-compatibility is also not an issue for certain uses (such as ours), we build with LTCG against libzmq-static anyway.

It's not only the platform - with such a class, even changing compiler could mean an ABI breakage. We have to be very careful unfortunately.

Anyway, now I remember why the content_t cannot be inlined - it's because the way sharing large messages works, for example when PUB sends the same message to multiple SUBs. Each msg_t shares the same content_t, so that the data doesn't have to be copied, improving performances really a lot. But to safely refcount it, the counter needs to be shared too - otherwise it doesn't work. So content_t has to be unique as well. If it were inlined in msg_t, it wouldn't be - each shallow copy of the message would have its own counter.

In the v2 decoder this is further optimised since 4.2 by batching these buffers - they are allocated in 18KB blocks (8KB for data, the rest for content_t, enough to store the minimium-size large messages that fit 8KB).
If you have very specific workloads, you can tweak this numbers in src/config.h.

BTW that 4.2 change caused a lot of problems, even though it's an amazing performance improvement - twice as fast in some cases. I absolutely love it. But because of the batching, individual messages data pointers are no longer aligned. Until 4.1 they were aligned by chance, since each data buffer was malloc'ed and on most implementations that means it returns an aligned address. Now this was not part of the public API/ABI promise, was not documented in any way, and happened "by chance" due to the way glibc and the kernel implement memory allocation. And yet there was a huge backlash from certain application developers who were relying on this undocumented behaviour. It caused lots of problems and weeks of work, almost missed Debian 9's deadline due to this.

That's why I try to be very very careful... even the simplest change in behaviour might cause real issues for users. Once something is part of the public contract it has to be maintained. That's the price to pay when maintaining an old and popular library I'm afraid.

Would you find it acceptable if I added such a function, of course as DRAFT API? Before un-DRAFTing it, this can be thoroughly reconsidered.

Of course, you can send all the changes you like :-) We just need to be careful when releasing, that's all.

[sigiesec]

That's really interesting. I was not aware of the batch allocation. I just noticed in a performance test of our component that the content_t malloc was the malloc called most often. I will recheck that before working on the suggested extension.

In general I think it is essential to have CI tests for any property that is guaranteed to the users, be it functional or non-functional properties. But I can imagine that there is no free performance test environment.

[bluca]

That's really interesting. I was not aware of the batch allocation. I just noticed in a performance test of our component that the content_t malloc was the malloc called most often. I will recheck that before working on the suggested extension.

Note that this is on the receive side. If you have performance test available that exercise the recv side, try fiddling with config.h - you should see great improvements. If the workload is deterministic there are big gains to be made.

In general I think it is essential to have CI tests for any property that is guaranteed to the users, be it functional or non-functional properties. But I can imagine that there is no free performance test environment.

Yes absolutely. There are performance tests in the repo actually, that measure a few things, perhaps adding something like "make perf" would be a nice addition, then we can run them on Travis.
I guess the problem is that those VMs are so overbooked that I'm not sure how realistic it would be, given over localhost/ipc there's heavy I/O bottlenecks...

[bluca]

Note that this is on the receive side. If you have performance test available that exercise the recv side, try fiddling with config.h - you should see great improvements. If the workload is deterministic there are big gains to be made.

On the send side, given each message is allocated by the applitcation thread, this batching doesn't happen.

But perhaps that would be a good idea for improvement - try to come up with a way to pre- or batch-allocate in zmq_msg_init_data. This would avoid having to further complicate the API.
The problem to solve is thread safety - it cannot use thread-local storage as messages are handed over to the I/O thread.
Perhaps an atomic pointer to the next free element and memory barriers.

The obvious advantage it that it would be completely transparent, so if it needs new compiler constructs it can be achieved with backward-compatibility by falling back to the old malloc-per-msg behaviour.

Other issues to keep in mind:

1. pointer-size alignment, on some platform breaking it causes SIGBUS
2. pointer-size alignment of data buffers, same as the receive problem