Re: [PATCH v1 1/2] migration/xbzrle: replace transferred xbzrle bytes with encoded bytes

[Wei Wang <wei.w.wang@intel.com>]

On 04/22/2020 03:21 AM, Dr. David Alan Gilbert wrote:
> * Wei Wang (wei.w.wang@intel.com) wrote:
>> Like compressed_size which indicates how many bytes are compressed, we
>> need encoded_size to understand how many bytes are encoded with xbzrle
>> during migration.
>>
>> Replace the old xbzrle_counter.bytes, instead of adding a new counter,
>> because we don't find a usage of xbzrle_counter.bytes currently, which
>> includes 3 more bytes of the migration transfer protocol header (in
>> addition to the encoding header). The encoded_size will further be used
>> to calculate the encoding rate.
>>
>> Signed-off-by: Yi Sun <yi.y.sun@intel.com>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> Can you explain why these 3 bytes matter?  Certainly the 2 bytes of the
> encoded_len are an overhead that's a cost of using XBZRLE; so if you're
> trying to figure out whether xbzrle is worth it, then you should include
> those 2 bytes in the cost.
> That other byte, that holds ENCODING_FLAG_XBZRLE also seems to be pure
> oerhead of XBZRLE; so your cost of using XBZRLE really does include
> those 3 bytes.
>
> SO to me it makes sense to include the 3 bytes as it currently does.
>
> Dave

Thanks Dave for sharing your thoughts.

We hope to do a fair comparison of compression rate and xbzrle encoding 
rate.
The current compression_rate doesn't include the migration flag overhead 
(please see
update_compress thread_counts() ). So for xbzrle encoding rate, we 
wanted it not include the migration
protocol flags as well (but the 2 bytes xbzrle encoding overhead is kept 
there, as the compression rate
includes the compression header overhead).

Or would you think it is necessary to add the migration flag (8 bytes) 
for compression
when calculating the compression rate?

Best,
Wei