[Spice-devel] [spice-gtk PATCH] Usbredir: enable lz4 compression
Snir Sheriber
ssheribe at redhat.com
Thu Apr 7 14:38:41 UTC 2016
On 04/04/2016 11:48 AM, Victor Toso wrote:
> Hi,
>
> On Sun, Apr 03, 2016 at 06:40:09PM +0300, Snir Sheriber wrote:
>> Compressed message type is CompressedData which contains compression
>> type (1 byte) followed by the uncompressed data size (4 bytes) followed
>> by the compressed data size (4 bytes) followed by the compressed data
>>
>> If SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4 capability is available &&
>> data_size > COMPRESS_THRESHOLD data will be sent compressed otherwise
>> data will be sent uncompressed (as well if compression has failed)
>> ---
>> src/channel-usbredir.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 89 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
>> index d95a6c5..cd5cebd 100644
>> --- a/src/channel-usbredir.c
>> +++ b/src/channel-usbredir.c
>> @@ -24,6 +24,9 @@
>> #ifdef USE_USBREDIR
>> #include <glib/gi18n.h>
>> #include <usbredirhost.h>
>> +#ifdef USE_LZ4
>> +#include <lz4.h>
>> +#endif
>> #ifdef USE_POLKIT
>> #include "usb-acl-helper.h"
>> #endif
>> @@ -52,6 +55,7 @@
>>
>> #ifdef USE_USBREDIR
>>
>> +#define COMPRESS_THRESHOLD 1000
>> #define SPICE_USBREDIR_CHANNEL_GET_PRIVATE(obj) \
>> (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_USBREDIR_CHANNEL, SpiceUsbredirChannelPrivate))
>>
>> @@ -195,6 +199,8 @@ static void channel_set_handlers(SpiceChannelClass *klass)
>> {
>> static const spice_msg_handler handlers[] = {
>> [ SPICE_MSG_SPICEVMC_DATA ] = usbredir_handle_msg,
>> + [ SPICE_MSG_SPICEVMC_COMPRESSED_DATA ] = usbredir_handle_msg,
>> +
>> };
>>
>> spice_channel_set_handlers(klass, handlers, G_N_ELEMENTS(handlers));
>> @@ -231,6 +237,9 @@ void spice_usbredir_channel_set_context(SpiceUsbredirChannel *channel,
>> #if USBREDIR_VERSION >= 0x000701
>> usbredirhost_set_buffered_output_size_cb(priv->host, usbredir_buffered_output_size_callback);
>> #endif
>> +#ifdef USE_LZ4
>> + spice_channel_set_capability(channel, SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4);
>> +#endif
>> }
>>
>> static gboolean spice_usbredir_channel_open_device(
>> @@ -544,11 +553,52 @@ static void usbredir_free_write_cb_data(uint8_t *data, void *user_data)
>> usbredirhost_free_write_buffer(priv->host, data);
>> }
>>
>> +#ifdef USE_LZ4
>> +static void usbredir_free_write_cb_compressed_data(uint8_t *data, void *user_data)
>> +{
>> + SpiceUsbredirChannel *channel = user_data;
>> + SpiceUsbredirChannelPrivate *priv = channel->priv;
>> +
>> + usbredirhost_free_write_buffer(priv->host, (uint8_t*)SPICE_CONTAINEROF(data, SpiceMsgCompressedData, compressed_data));
>> +}
>> +#endif
>> +
>> static int usbredir_write_callback(void *user_data, uint8_t *data, int count)
>> {
>> SpiceUsbredirChannel *channel = user_data;
>> SpiceMsgOut *msg_out;
>> -
>> +#ifdef USE_LZ4
>> + int bound;
>> +
>> + if(count > COMPRESS_THRESHOLD &&
>> + spice_channel_test_capability(SPICE_CHANNEL(channel),
>> + SPICE_SPICEVMC_CAP_DATA_COMPRESS_LZ4) &&
>> + ((bound = LZ4_compressBound(count)) != 0)) {
> So, in this case, you are compressing all outgoing data in channel-usbredir.
>
> I'm not 100% positive but I don't think we should compress data from isoc
> devices. They should already be compressed. What do you think?
>
> For other types of data in channel-usbredir and channel-webdav this is fine and
> probably would safe a great deal of bandwidth. Have you done any measurements to
> check the gains?
>
> Cheers,
> toso
Yes, you are right, when compressing data which is already compressed
the compression ratio is not high (for example with the camera device),
I'll see how can i identify isoc devices and maybe compress only those.
I've made some testing- when compressing usb storage device with ntfs
fs it's mostly depends on the files which are being transfer, the
compression ratio can be 9:10 for compressed\binary files and up to 1:9
for documents\code\other files (and during mounting empty\not-full fs-
the compression ratio is always very high since a lot of "empty" data is
being sent). so basically the results seems pretty good for file systems
(with the camera device the compression ratio was not high)
The perf testing has shown that the cpu is not being significantly
influenced by the compression.
>> + int compressed_data_count;
>> + SpiceMsgCompressedData *compressed_data_msg;
>> +
>> + compressed_data_msg = (SpiceMsgCompressedData*)spice_malloc(sizeof(SpiceMsgCompressedData) + bound);
>> + compressed_data_msg->uncompressed_size = count;
>> + compressed_data_msg->type = SPICE_DATA_COMPRESSION_TYPE_LZ4;
>> + compressed_data_count = LZ4_compress_default((char*)data,
>> + (char*)compressed_data_msg->compressed_data, count, bound);
>> + if (compressed_data_count > 0) {
>> + compressed_data_msg->compressed_size = compressed_data_count;
>> + msg_out = spice_msg_out_new(SPICE_CHANNEL(channel),
>> + SPICE_MSGC_SPICEVMC_COMPRESSED_DATA);
>> + msg_out->marshallers->msg_SpiceMsgCompressedData(msg_out->marshaller, compressed_data_msg);
>> + spice_marshaller_add_ref_full(msg_out->marshaller,
>> + (uint8_t*)compressed_data_msg->compressed_data,
>> + compressed_data_count,
>> + usbredir_free_write_cb_compressed_data,
>> + channel);
>> + spice_msg_out_send(msg_out);
>> + return count;
>> + } else {/* free & fallback to sending the message uncompressed */
>> + free(compressed_data_msg);
>> + }
>> + }
>> +#endif
>> msg_out = spice_msg_out_new(SPICE_CHANNEL(channel),
>> SPICE_MSGC_SPICEVMC_DATA);
>> spice_marshaller_add_ref_full(msg_out->marshaller, data, count,
>> @@ -639,19 +689,51 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)
>> SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c);
>> SpiceUsbredirChannelPrivate *priv = channel->priv;
>> device_error_data data;
>> - int r, size;
>> - uint8_t *buf;
>> + int r = 0;
>>
>> g_return_if_fail(priv->host != NULL);
>>
>> /* No recursion allowed! */
>> g_return_if_fail(priv->read_buf == NULL);
>>
>> - buf = spice_msg_in_raw(in, &size);
>> - priv->read_buf = buf;
>> - priv->read_buf_size = size;
>> + if (spice_msg_in_type(in) == SPICE_MSG_SPICEVMC_COMPRESSED_DATA) {
>> + SpiceMsgCompressedData *compressed_data_msg = spice_msg_in_parsed(in);
>> + uint32_t decompressed_size = 0;
>> + char* decompressed;
>> +
>> + switch(compressed_data_msg->type) {
>> +#ifdef USE_LZ4
>> + case SPICE_DATA_COMPRESSION_TYPE_LZ4:
>> + decompressed = (char*)malloc(compressed_data_msg->uncompressed_size);
>> + decompressed_size = LZ4_decompress_safe ((char*)compressed_data_msg->compressed_data,
>> + decompressed,
>> + compressed_data_msg->compressed_size,
>> + compressed_data_msg->uncompressed_size);
>> + break;
>> +#endif
>> + default:
>> + decompressed = NULL;
>> + g_warning("Unknown Compression Type");
>> + r = usbredirhost_read_parse_error;
>> + }
>> + if (decompressed_size != compressed_data_msg->uncompressed_size) {
>> + g_warning("Decompress Error decompressed_size=%d expected=%d",
>> + decompressed_size, compressed_data_msg->uncompressed_size);
>> + r = usbredirhost_read_parse_error;
>> + } else {
>> + priv->read_buf_size = decompressed_size;
>> + priv->read_buf = (uint8_t*)decompressed;
>> + }
>> + } else { /*Regular SPICE_MSG_SPICEVMC_DATA msg*/
>> + int size;
>> + uint8_t *buf;
>> + buf = spice_msg_in_raw(in, &size);
>> + priv->read_buf_size = size;
>> + priv->read_buf = buf;
>> + }
>>
>> - r = usbredirhost_read_guest_data(priv->host);
>> + if (r == 0)
>> + r = usbredirhost_read_guest_data(priv->host);
>> if (r != 0) {
>> SpiceUsbDevice *spice_device = priv->spice_device;
>> gchar *desc;
>> --
>> 2.4.11
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160407/ef390e15/attachment.html>
More information about the Spice-devel
mailing list