Message type optimizations

[newpavlov]

I think wayland_commons::wire::Message is a bit sub-optimal right now.

Currently Vec is allocated for every message, while judging by wayland.xml max number of arguments is 8. To remove unnecessary allocations we can use SmallVec for args, though we will have to decide the array size for SmallVec, e.g. 3, 4, 6, or 8.

Argument always heap allocates for Array and String types, not only it causes needless allocations and data copies, but also makes Argument bigger (right now it uses 32 bytes). Instead of copying data we could point to the origin buffer instead, i.e. we will add a lifetime Argument<'a> bound to the origin buffer (and consequently to Message as well). The simplest option will be to use &'a [u8] and &'a CStr, but it will bring Argument size to only 24 bytes. Instead with a bit of unsafe code we can store *const u8 and phantom data (to track lifetime) inside two opaque types which will dereference to &'a [u8] and &'a CStr respectively. It will bring Argument size to 16 bytes on 64-bit machines. I think adding lifetime to Message shouldn't be a big problem, as IIUC they are almost immediately consumed inside event loop.

[elinorbgr]

Currently Vec is allocated for every message, while judging by wayland.xml max number of arguments is 8. To remove unnecessary allocations we can use SmallVec for args, though we will have to decide the array size for SmallVec, e.g. 3, 4, 6, or 8.

Agreed, most messages have less than 4 arguments in general, though 8 is not some hard limit, other protocol extensions can definitely have messages with more arguments, nothing forbids that. And using SmallVec here seems to be a simple enough change.

I think adding lifetime to Message shouldn't be a big problem, as IIUC they are almost immediately consumed inside event loop.

Well, yes and no. Server-side this is mostly true, but client side it isn't. Messages are first stored in temporary buffers for each event queue, and there is no guarantee that the messages will be consumed from these event queues.

In a more general view, I'm not very clear about what would be gained from such optimizations. In contrast, the C library also does a lot of copying around and allocations, and it does not appear to be such a bottleneck that anyone has complained about it. So I'm not willing to significantly increase the complexity of the implementation for small hypothetical performance gains.

[newpavlov]

Messages are first stored in temporary buffers for each event queue

Hm, then one way to solve this will be to introduce in addition to the current owning Message a borrowing MessageRef<'a>, with an ability to easily get one from another. But if you'll judge it's too much trouble for too little gain, feel free to ignore this proposal. It's just that I found such copies and allocations a bit wasteful, that is all. :) Also making Argument twice smaller will make Message smaller as well. Maybe having statistics for passed messages for common usage patterns and applications could be useful, it will show number of strings and arrays flying around and ratio of messages with number of arguments bigger than 4 (or whatever will be chosen for SmallVec).

[elinorbgr]

Reducing the allocations using SmallVec will likely be a significant gain indeed, as there can be a relatively high frequency of exchanged messages at times.

Regarding copies though, in practice messages with String or Array arguments are rather rare in proportion to be honest. A few of them are typically exchanged during the initialization of a client, and occasionally during the run (for example when a window gains focus), but that's about it. So I don't think there is much to be gained here honestly.

Though I'll seriously consider the issue if somebody ever reports how this is a bottleneck for their specific app, but other than that I prefer to keep the implementation as simple as possible (which is already not very simple 😕 ).