Re: [PATCH v3] migration/xbzrle: add encoding rate

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

On 06/04/2020 05:38 PM, Dr. David Alan Gilbert wrote:
> * Richard Henderson (richard.henderson@linaro.org) wrote:
>> On 6/3/20 7:58 PM, Wei Wang wrote:
>>> It is possible that encoded_size==0, but unencoded_size !=0. For example,
>>> a page is written with the same data that it already has.
>> That really contains 0 bytes?
>> Not even the ones that say "same data"?
>>
>> You certainly have a magical compression algorithm there.
>> Or bad accounting.
> We just don't bother sending the page at all in the case it's not
> changed; no headers, no nothing:
>
>      if (encoded_len == 0) {
>          trace_save_xbzrle_page_skipping();
>          return 0;
>
> and that's xbzrle having correctly done it's job.
>
>
>>> The encoding_rate is expected to reflect if the page is xbzrle encoding friendly.
>>> The larger, the more friendly, so 0 might not be a good representation here.
>>>
>>> Maybe, we could change UINT64_MAX above to "~0ULL" to avoid the issue?
>> ~0ull is no different than UINT64_MAX -- indeed, they are *exactly* the same
>> value -- and is not an exactly representible floating-point value.
>>
>> If unencoded_size != 0, and (somehow) encoded_size == 0, then
>>
>>    unencoded_size / encoded_size = Inf
>>
>> which is indeed the limit of x -> 0, n / x.
>>
>> Which is *also* printable by %0.2f.
>>
>> I still contend that the middle if should be removed, and you should print out
>> whatever's left.  Either NaN or Inf is instructive.  Certainly nothing in the
>> middle cares about the actual value.
> Hmm OK; I'll admit to not liking NaN/Inf in output.
>
> Dave
>

OK. To deal with the reported issue, how about using FLT_MAX (as opposed 
to UINT64_MAX or inf):
xbzrle_counters.encoding_rate = FLT_MAX;


Best,
Wei