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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* 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

> 
> r~
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK