* Wei Wang (wei.w.w...@intel.com) wrote:
> On 04/28/2020 10:51 PM, Dr. David Alan Gilbert wrote:
> > * Wei Wang (wei.w.w...@intel.com) wrote:
> > > Users may need to check the xbzrle encoding rate to know if the guest
> > > memory is xbzrle encoding-friendly, and dynamically turn off the
> > > encoding if the encoding rate is low.
> > >
> > > Signed-off-by: Yi Sun <yi.y....@intel.com>
> > > Signed-off-by: Wei Wang <wei.w.w...@intel.com>
> > > ---
> > > migration/migration.c | 1 +
> > > migration/ram.c | 38 ++++++++++++++++++++++++++++++++++++--
> > > monitor/hmp-cmds.c | 2 ++
> > > qapi/migration.json | 5 ++++-
> > > 4 files changed, 43 insertions(+), 3 deletions(-)
> > >
> > > ChangeLog:
> > > - include the 3 bytes (ENCODING_FLAG_XBZRLE flag and encoded_len) when
> > > calculating the encoding rate. Similar to the compress rate
> > > calculation, the 8 byte RAM_SAVE_FLAG_CONTINUE flag isn't included in
> > > the calculation.
> > >
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index 187ac04..e404213 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -930,6 +930,7 @@ static void populate_ram_info(MigrationInfo *info,
> > > MigrationState *s)
> > > info->xbzrle_cache->pages = xbzrle_counters.pages;
> > > info->xbzrle_cache->cache_miss = xbzrle_counters.cache_miss;
> > > info->xbzrle_cache->cache_miss_rate =
> > > xbzrle_counters.cache_miss_rate;
> > > + info->xbzrle_cache->encoding_rate =
> > > xbzrle_counters.encoding_rate;
> > > info->xbzrle_cache->overflow = xbzrle_counters.overflow;
> > > }
> > > diff --git a/migration/ram.c b/migration/ram.c
> > > index 04f13fe..f46ab96 100644
> > > --- a/migration/ram.c
> > > +++ b/migration/ram.c
> > > @@ -327,6 +327,10 @@ struct RAMState {
> > > uint64_t num_dirty_pages_period;
> > > /* xbzrle misses since the beginning of the period */
> > > uint64_t xbzrle_cache_miss_prev;
> > > + /* Amount of xbzrle pages since the beginning of the period */
> > > + uint64_t xbzrle_pages_prev;
> > > + /* Amount of xbzrle encoded bytes since the beginning of the period
> > > */
> > > + uint64_t xbzrle_bytes_prev;
> > > /* compression statistics since the beginning of the period */
> > > /* amount of count that no free thread to compress data */
> > > @@ -696,6 +700,18 @@ static int save_xbzrle_page(RAMState *rs, uint8_t
> > > **current_data,
> > > return -1;
> > > }
> > > + /*
> > > + * Reaching here means the page has hit the xbzrle cache, no matter
> > > what
> > > + * encoding result it is (normal encoding, overflow or skipping the
> > > page),
> > > + * count the page as encoded. This is used to caculate the encoding
> > > rate.
> > > + *
> > > + * Example: 2 pages (8KB) being encoded, first page encoding
> > > generates 2KB,
> > > + * 2nd page turns out to be skipped (i.e. no new bytes written to the
> > > + * page), the overall encoding rate will be 8KB / 2KB = 4, which has
> > > the
> > > + * skipped page included. In this way, the encoding rate can tell if
> > > the
> > > + * guest page is good for xbzrle encoding.
> > > + */
> > > + xbzrle_counters.pages++;
> > Can you explain how that works with overflow? Doesn't overflow return
> > -1 here, not increment the bytes, so it looks like you've xbzrle'd a
> > page, but the encoding rate hasn't incremented.
>
> OK. How about adding below before returning -1 (for the overflow case):
>
> ...
> xbzrle_counters.bytes += TARGET_PAGE_SIZE;
> return -1;
Yes, I think that feels better.
Dave
> Example: if we have 2 pages encoded as below:
> 4KB--> after normal encoding: 2KB
> 4KB--> after overflow: 4KB (will be sent as non-encoded page)
> then the encoding rate is 8KB / 6KB = ~1.3
> (if we skip the counting of the overflow case,
> the encoding rate will be 4KB/ 2KB=2. Users may think that's
> good to go with xbzrle, unless they do another calculation with
> checking the overflow numbers themselves)
>
> Best,
> Wei
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
