diff mbox series

[v2] migration/xbzrle: add encoding rate

Message ID 1587974511-14953-1-git-send-email-wei.w.wang@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] migration/xbzrle: add encoding rate | expand

Commit Message

Wang, Wei W April 27, 2020, 8:01 a.m. UTC
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.sun@intel.com>
Signed-off-by: Wei Wang <wei.w.wang@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.

Comments

Dr. David Alan Gilbert April 28, 2020, 2:51 p.m. UTC | #1
* Wei Wang (wei.w.wang@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.sun@intel.com>
> Signed-off-by: Wei Wang <wei.w.wang@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.

Dave

>      prev_cached_page = get_cached_data(XBZRLE.cache, current_addr);
>  
>      /* save current buffer into memory */
> @@ -736,8 +752,12 @@ static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
>      qemu_put_be16(rs->f, encoded_len);
>      qemu_put_buffer(rs->f, XBZRLE.encoded_buf, encoded_len);
>      bytes_xbzrle += encoded_len + 1 + 2;
> -    xbzrle_counters.pages++;
> -    xbzrle_counters.bytes += bytes_xbzrle;
> +    /*
> +     * Like compressed_size (please see update_compress_thread_counts),
> +     * the xbzrle encoded bytes don't count the 8 byte header with
> +     * RAM_SAVE_FLAG_CONTINUE.
> +     */
> +    xbzrle_counters.bytes += bytes_xbzrle - 8;
>      ram_counters.transferred += bytes_xbzrle;
>  
>      return 1;
> @@ -870,9 +890,23 @@ static void migration_update_rates(RAMState *rs, int64_t end_time)
>      }
>  
>      if (migrate_use_xbzrle()) {
> +        double encoded_size, unencoded_size;
> +
>          xbzrle_counters.cache_miss_rate = (double)(xbzrle_counters.cache_miss -
>              rs->xbzrle_cache_miss_prev) / page_count;
>          rs->xbzrle_cache_miss_prev = xbzrle_counters.cache_miss;
> +        unencoded_size = (xbzrle_counters.pages - rs->xbzrle_pages_prev) *
> +                         TARGET_PAGE_SIZE;
> +        encoded_size = xbzrle_counters.bytes - rs->xbzrle_bytes_prev;
> +        if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
> +            xbzrle_counters.encoding_rate = 0;
> +        } else if (!encoded_size) {
> +            xbzrle_counters.encoding_rate = UINT64_MAX;
> +        } else {
> +            xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
> +        }
> +        rs->xbzrle_pages_prev = xbzrle_counters.pages;
> +        rs->xbzrle_bytes_prev = xbzrle_counters.bytes;
>      }
>  
>      if (migrate_use_compression()) {
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 9b94e67..c2a3a66 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -303,6 +303,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>                         info->xbzrle_cache->cache_miss);
>          monitor_printf(mon, "xbzrle cache miss rate: %0.2f\n",
>                         info->xbzrle_cache->cache_miss_rate);
> +        monitor_printf(mon, "xbzrle encoding rate: %0.2f\n",
> +                       info->xbzrle_cache->encoding_rate);
>          monitor_printf(mon, "xbzrle overflow: %" PRIu64 "\n",
>                         info->xbzrle_cache->overflow);
>      }
> diff --git a/qapi/migration.json b/qapi/migration.json
> index eca2981..358e402 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -70,6 +70,8 @@
>  #
>  # @cache-miss-rate: rate of cache miss (since 2.1)
>  #
> +# @encoding-rate: rate of encoded bytes (since 5.1)
> +#
>  # @overflow: number of overflows
>  #
>  # Since: 1.2
> @@ -77,7 +79,7 @@
>  { 'struct': 'XBZRLECacheStats',
>    'data': {'cache-size': 'int', 'bytes': 'int', 'pages': 'int',
>             'cache-miss': 'int', 'cache-miss-rate': 'number',
> -           'overflow': 'int' } }
> +           'encoding-rate': 'number', 'overflow': 'int' } }
>  
>  ##
>  # @CompressionStats:
> @@ -337,6 +339,7 @@
>  #             "pages":2444343,
>  #             "cache-miss":2244,
>  #             "cache-miss-rate":0.123,
> +#             "encoding-rate":80.1,
>  #             "overflow":34434
>  #          }
>  #       }
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Wang, Wei W April 29, 2020, 1 a.m. UTC | #2
On 04/28/2020 10:51 PM, Dr. David Alan Gilbert wrote:
> * Wei Wang (wei.w.wang@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.sun@intel.com>
>> Signed-off-by: Wei Wang <wei.w.wang@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;

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 April 29, 2020, 11 a.m. UTC | #3
* Wei Wang (wei.w.wang@intel.com) wrote:
> On 04/28/2020 10:51 PM, Dr. David Alan Gilbert wrote:
> > * Wei Wang (wei.w.wang@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.sun@intel.com>
> > > Signed-off-by: Wei Wang <wei.w.wang@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 / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

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++;
     prev_cached_page = get_cached_data(XBZRLE.cache, current_addr);
 
     /* save current buffer into memory */
@@ -736,8 +752,12 @@  static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
     qemu_put_be16(rs->f, encoded_len);
     qemu_put_buffer(rs->f, XBZRLE.encoded_buf, encoded_len);
     bytes_xbzrle += encoded_len + 1 + 2;
-    xbzrle_counters.pages++;
-    xbzrle_counters.bytes += bytes_xbzrle;
+    /*
+     * Like compressed_size (please see update_compress_thread_counts),
+     * the xbzrle encoded bytes don't count the 8 byte header with
+     * RAM_SAVE_FLAG_CONTINUE.
+     */
+    xbzrle_counters.bytes += bytes_xbzrle - 8;
     ram_counters.transferred += bytes_xbzrle;
 
     return 1;
@@ -870,9 +890,23 @@  static void migration_update_rates(RAMState *rs, int64_t end_time)
     }
 
     if (migrate_use_xbzrle()) {
+        double encoded_size, unencoded_size;
+
         xbzrle_counters.cache_miss_rate = (double)(xbzrle_counters.cache_miss -
             rs->xbzrle_cache_miss_prev) / page_count;
         rs->xbzrle_cache_miss_prev = xbzrle_counters.cache_miss;
+        unencoded_size = (xbzrle_counters.pages - rs->xbzrle_pages_prev) *
+                         TARGET_PAGE_SIZE;
+        encoded_size = xbzrle_counters.bytes - rs->xbzrle_bytes_prev;
+        if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
+            xbzrle_counters.encoding_rate = 0;
+        } else if (!encoded_size) {
+            xbzrle_counters.encoding_rate = UINT64_MAX;
+        } else {
+            xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
+        }
+        rs->xbzrle_pages_prev = xbzrle_counters.pages;
+        rs->xbzrle_bytes_prev = xbzrle_counters.bytes;
     }
 
     if (migrate_use_compression()) {
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9b94e67..c2a3a66 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -303,6 +303,8 @@  void hmp_info_migrate(Monitor *mon, const QDict *qdict)
                        info->xbzrle_cache->cache_miss);
         monitor_printf(mon, "xbzrle cache miss rate: %0.2f\n",
                        info->xbzrle_cache->cache_miss_rate);
+        monitor_printf(mon, "xbzrle encoding rate: %0.2f\n",
+                       info->xbzrle_cache->encoding_rate);
         monitor_printf(mon, "xbzrle overflow: %" PRIu64 "\n",
                        info->xbzrle_cache->overflow);
     }
diff --git a/qapi/migration.json b/qapi/migration.json
index eca2981..358e402 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -70,6 +70,8 @@ 
 #
 # @cache-miss-rate: rate of cache miss (since 2.1)
 #
+# @encoding-rate: rate of encoded bytes (since 5.1)
+#
 # @overflow: number of overflows
 #
 # Since: 1.2
@@ -77,7 +79,7 @@ 
 { 'struct': 'XBZRLECacheStats',
   'data': {'cache-size': 'int', 'bytes': 'int', 'pages': 'int',
            'cache-miss': 'int', 'cache-miss-rate': 'number',
-           'overflow': 'int' } }
+           'encoding-rate': 'number', 'overflow': 'int' } }
 
 ##
 # @CompressionStats:
@@ -337,6 +339,7 @@ 
 #             "pages":2444343,
 #             "cache-miss":2244,
 #             "cache-miss-rate":0.123,
+#             "encoding-rate":80.1,
 #             "overflow":34434
 #          }
 #       }