Support serialization of uint8 into broker::data about broker HOT 5 CLOSED

Instead of (mis)using an integer type, I think we should add the alias using byte_buffer = std::vector<byte> to Broker and then extend the data variant so that it can hold a buffer.

from broker.

I am not sorry that this actually is a misuse in the cases that we are using. It just actually is a ton of uint8 data (which actually are 8 bit integers that are used in the code as such).

I am open to casting them to something else - but just being able to store uint8s would be nice as well. Unless there is a reason to not support this.

from broker.

actually are 8 bit integers that are used in the code as such

At first reading, I thought this was about serializing data into 8-bit integers and then storing the buffer inside a data. Hence my suggestion to use a more appropriately typed buffer. 🙂

Reading your comment (and the original post again): Simply adding uint8_t to the data variant would make working with this type definitely more complicated. The original design idea was to have only one type per domain to make working with the variant straightforward. Since variants always have the size of the largest type, there's no performance benefit from including smaller integer types (adding more types may even slow down matching with apply since it has to consider more cases).

It's also a breaking API change. So I think the question is whether saving bytes on the wire translates to a significant boost in performance that makes breaking the API (and making it more cumbersome to work with) worthwhile.

from broker.

Ok, I actually never looked into the implementation of this - and was not aware that this always has the size of the largest member type (so will always be 64 bits I guess).

In this case - you are right - adding this does not make any sense.

It still is a bit sad - since I guess this leaves us with the choice of transmitting 8 times as much data as necessary - or complicating the serialization/deserialization code.

from broker.

In this case - you are right - adding this does not make any sense.

Closing it, then. 🙂

from broker.