Re: [Qemu-devel] [PATCH 05/12] migration: show the statistics of compression
On 07/17/2018 03:01 AM, Dr. David Alan Gilbert wrote:
* Xiao Guangrong (guangrong.x...@gmail.com) wrote:
On 06/14/2018 12:25 AM, Dr. David Alan Gilbert wrote:
}
static void migration_bitmap_sync(RAMState *rs)
@@ -1412,6 +1441,9 @@ static void flush_compressed_data(RAMState *rs)
qemu_mutex_lock(&comp_param[idx].mutex);
if (!comp_param[idx].quit) {
len = qemu_put_qemu_file(rs->f, comp_param[idx].file);
+/* 8 means a header with RAM_SAVE_FLAG_CONTINUE. */
+compression_counters.reduced_size += TARGET_PAGE_SIZE - len + 8;
I think I'd rather save just len+8 rather than than the subtraction.
Hmm, is this what you want?
compression_counters.reduced_size += len - 8;
Then calculate the real reduced size in populate_ram_info() where we return this
info to the user:
info->compression->reduced_size = compression_counters.pages * PAGE_SIZE
- compression_counters.reduced_size;
Right?
I mean I'd rather see the actual size presented to the user rather than
the saving compared to uncompressed.
These statistics are used to help people to see whether compression works
efficiently or not,
so maybe reduced-size is more straightforward? :)
Re: [Qemu-devel] [PATCH 05/12] migration: show the statistics of compression
* Xiao Guangrong (guangrong.x...@gmail.com) wrote:
>
>
> On 06/14/2018 12:25 AM, Dr. David Alan Gilbert wrote:
> }
> > > static void migration_bitmap_sync(RAMState *rs)
> > > @@ -1412,6 +1441,9 @@ static void flush_compressed_data(RAMState *rs)
> > > qemu_mutex_lock(&comp_param[idx].mutex);
> > > if (!comp_param[idx].quit) {
> > > len = qemu_put_qemu_file(rs->f, comp_param[idx].file);
> > > +/* 8 means a header with RAM_SAVE_FLAG_CONTINUE. */
> > > +compression_counters.reduced_size += TARGET_PAGE_SIZE - len
> > > + 8;
> >
> > I think I'd rather save just len+8 rather than than the subtraction.
> >
> Hmm, is this what you want?
> compression_counters.reduced_size += len - 8;
>
> Then calculate the real reduced size in populate_ram_info() where we return
> this
> info to the user:
> info->compression->reduced_size = compression_counters.pages *
> PAGE_SIZE - compression_counters.reduced_size;
>
> Right?
I mean I'd rather see the actual size presented to the user rather than
the saving compared to uncompressed.
Dave
> > I think other than that, and Eric's comments, it's OK.
> >
>
> Thanks.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 05/12] migration: show the statistics of compression
On 06/14/2018 12:25 AM, Dr. David Alan Gilbert wrote:
}
static void migration_bitmap_sync(RAMState *rs)
@@ -1412,6 +1441,9 @@ static void flush_compressed_data(RAMState *rs)
qemu_mutex_lock(&comp_param[idx].mutex);
if (!comp_param[idx].quit) {
len = qemu_put_qemu_file(rs->f, comp_param[idx].file);
+/* 8 means a header with RAM_SAVE_FLAG_CONTINUE. */
+compression_counters.reduced_size += TARGET_PAGE_SIZE - len + 8;
I think I'd rather save just len+8 rather than than the subtraction.
Hmm, is this what you want?
compression_counters.reduced_size += len - 8;
Then calculate the real reduced size in populate_ram_info() where we return this
info to the user:
info->compression->reduced_size = compression_counters.pages * PAGE_SIZE
- compression_counters.reduced_size;
Right?
I think other than that, and Eric's comments, it's OK.
Thanks.
Re: [Qemu-devel] [PATCH 05/12] migration: show the statistics of compression
* guangrong.x...@gmail.com (guangrong.x...@gmail.com) wrote:
> From: Xiao Guangrong
>
> Then the uses can adjust the parameters based on this info
>
> Currently, it includes:
> pages: amount of pages compressed and transferred to the target VM
> busy: amount of count that no free thread to compress data
> busy-rate: rate of thread busy
> reduced-size: amount of bytes reduced by compression
> compression-rate: rate of compressed size
>
> Signed-off-by: Xiao Guangrong
> ---
> hmp.c | 13 +
> migration/migration.c | 11 +++
> migration/ram.c | 37 +
> migration/ram.h | 1 +
> qapi/migration.json | 25 -
> 5 files changed, 86 insertions(+), 1 deletion(-)
>
> diff --git a/hmp.c b/hmp.c
> index ef93f4878b..5c2d3bd318 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -269,6 +269,19 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> info->xbzrle_cache->overflow);
> }
>
> +if (info->has_compression) {
> +monitor_printf(mon, "compression pages: %" PRIu64 " pages\n",
> + info->compression->pages);
> +monitor_printf(mon, "compression busy: %" PRIu64 "\n",
> + info->compression->busy);
> +monitor_printf(mon, "compression busy rate: %0.2f\n",
> + info->compression->busy_rate);
> +monitor_printf(mon, "compression reduced size: %" PRIu64 "\n",
> + info->compression->reduced_size);
> +monitor_printf(mon, "compression rate: %0.2f\n",
> + info->compression->compression_rate);
> +}
> +
> if (info->has_cpu_throttle_percentage) {
> monitor_printf(mon, "cpu throttle percentage: %" PRIu64 "\n",
> info->cpu_throttle_percentage);
> diff --git a/migration/migration.c b/migration/migration.c
> index 05aec2c905..bf7c63a5a2 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -693,6 +693,17 @@ static void populate_ram_info(MigrationInfo *info,
> MigrationState *s)
> info->xbzrle_cache->overflow = xbzrle_counters.overflow;
> }
>
> +if (migrate_use_compression()) {
> +info->has_compression = true;
> +info->compression = g_malloc0(sizeof(*info->compression));
> +info->compression->pages = compression_counters.pages;
> +info->compression->busy = compression_counters.busy;
> +info->compression->busy_rate = compression_counters.busy_rate;
> +info->compression->reduced_size = compression_counters.reduced_size;
> +info->compression->compression_rate =
> +compression_counters.compression_rate;
> +}
> +
> if (cpu_throttle_active()) {
> info->has_cpu_throttle_percentage = true;
> info->cpu_throttle_percentage = cpu_throttle_get_percentage();
> diff --git a/migration/ram.c b/migration/ram.c
> index ee03b28435..80914b747e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -292,6 +292,15 @@ struct RAMState {
> uint64_t num_dirty_pages_period;
> /* xbzrle misses since the beginning of the period */
> uint64_t xbzrle_cache_miss_prev;
> +
> +/* compression statistics since the beginning of the period */
> +/* amount of count that no free thread to compress data */
> +uint64_t compress_thread_busy_prev;
> +/* amount bytes reduced by compression */
> +uint64_t compress_reduced_size_prev;
> +/* amount of compressed pages */
> +uint64_t compress_pages_prev;
> +
> /* number of iterations at the beginning of period */
> uint64_t iterations_prev;
> /* Iterations since start */
> @@ -329,6 +338,8 @@ struct PageSearchStatus {
> };
> typedef struct PageSearchStatus PageSearchStatus;
>
> +CompressionStats compression_counters;
> +
> struct CompressParam {
> bool done;
> bool quit;
> @@ -1147,6 +1158,24 @@ static void migration_update_rates(RAMState *rs,
> int64_t end_time)
> rs->xbzrle_cache_miss_prev) / iter_count;
> rs->xbzrle_cache_miss_prev = xbzrle_counters.cache_miss;
> }
> +
> +if (migrate_use_compression()) {
> +uint64_t comp_pages;
> +
> +compression_counters.busy_rate = (double)(compression_counters.busy -
> +rs->compress_thread_busy_prev) / iter_count;
> +rs->compress_thread_busy_prev = compression_counters.busy;
> +
> +comp_pages = compression_counters.pages - rs->compress_pages_prev;
> +if (comp_pages) {
> +compression_counters.compression_rate =
> +(double)(compression_counters.reduced_size -
> +rs->compress_reduced_size_prev) /
> +(comp_pages * TARGET_PAGE_SIZE);
> +rs->compress_pages_prev = compression_counters.pages;
> +rs->compress_reduced_size_prev =
> compression_counters.reduced_size;
> +}
> +
Re: [Qemu-devel] [PATCH 05/12] migration: show the statistics of compression
On 06/05/2018 06:31 AM, Eric Blake wrote:
On 06/04/2018 04:55 AM, guangrong.x...@gmail.com wrote:
From: Xiao Guangrong
Then the uses can adjust the parameters based on this info
Currently, it includes:
pages: amount of pages compressed and transferred to the target VM
busy: amount of count that no free thread to compress data
busy-rate: rate of thread busy
reduced-size: amount of bytes reduced by compression
compression-rate: rate of compressed size
Signed-off-by: Xiao Guangrong
---
+++ b/qapi/migration.json
@@ -72,6 +72,26 @@
'cache-miss': 'int', 'cache-miss-rate': 'number',
'overflow': 'int' } }
+##
+# @CompressionStats:
+#
+# Detailed compression migration statistics
Sounds better as s/compression migration/migration compression/
Indeed.
+#
+# @pages: amount of pages compressed and transferred to the target VM
+#
+# @busy: amount of count that no free thread to compress data
Not sure what was meant, maybe:
@busy: count of times that no free thread was available to compress data
Yup, that's better.
+#
+# @busy-rate: rate of thread busy
In what unit? pages per second?
It's calculated by:
pages-directly-posted-out-without-compression / total-page-posted-out
+#
+# @reduced-size: amount of bytes reduced by compression
+#
+# @compression-rate: rate of compressed size
In what unit?
It's calculated by:
size-posted-out-after-compression / (compressed-page * page_size, i.e, that
is
the raw data without compression)
+#
+##
Missing a 'Since: 3.0' tag
Wow, directly upgrade to 3.0, big step. :-)
Will add this tag in the next version.
+{ 'struct': 'CompressionStats',
+ 'data': {'pages': 'int', 'busy': 'int', 'busy-rate': 'number',
+ 'reduced-size': 'int', 'compression-rate': 'number' } }
+
##
# @MigrationStatus:
#
@@ -169,6 +189,8 @@
# only present when the postcopy-blocktime migration capability
# is enabled. (Since 2.13)
Pre-existing - we need to fix this 2.13 to be 3.0 (if it isn't already fixed)
I should re-sync the repo before making patches next time.
#
+# @compression: compression migration statistics, only returned if compression
+# feature is on and status is 'active' or 'completed' (Since 2.14)
There will not be a 2.14 (for that matter, not even a 2.13). The next release
is 3.0.
Okay, will fix.
Re: [Qemu-devel] [PATCH 05/12] migration: show the statistics of compression
On 06/04/2018 04:55 AM, guangrong.x...@gmail.com wrote:
From: Xiao Guangrong
Then the uses can adjust the parameters based on this info
Currently, it includes:
pages: amount of pages compressed and transferred to the target VM
busy: amount of count that no free thread to compress data
busy-rate: rate of thread busy
reduced-size: amount of bytes reduced by compression
compression-rate: rate of compressed size
Signed-off-by: Xiao Guangrong
---
+++ b/qapi/migration.json
@@ -72,6 +72,26 @@
'cache-miss': 'int', 'cache-miss-rate': 'number',
'overflow': 'int' } }
+##
+# @CompressionStats:
+#
+# Detailed compression migration statistics
Sounds better as s/compression migration/migration compression/
+#
+# @pages: amount of pages compressed and transferred to the target VM
+#
+# @busy: amount of count that no free thread to compress data
Not sure what was meant, maybe:
@busy: count of times that no free thread was available to compress data
+#
+# @busy-rate: rate of thread busy
In what unit? pages per second?
+#
+# @reduced-size: amount of bytes reduced by compression
+#
+# @compression-rate: rate of compressed size
In what unit?
+#
+##
Missing a 'Since: 3.0' tag
+{ 'struct': 'CompressionStats',
+ 'data': {'pages': 'int', 'busy': 'int', 'busy-rate': 'number',
+ 'reduced-size': 'int', 'compression-rate': 'number' } }
+
##
# @MigrationStatus:
#
@@ -169,6 +189,8 @@
# only present when the postcopy-blocktime migration capability
# is enabled. (Since 2.13)
Pre-existing - we need to fix this 2.13 to be 3.0 (if it isn't already
fixed)
#
+# @compression: compression migration statistics, only returned if compression
+# feature is on and status is 'active' or 'completed' (Since 2.14)
There will not be a 2.14 (for that matter, not even a 2.13). The next
release is 3.0.
#
# Since: 0.14.0
##
@@ -183,7 +205,8 @@
'*cpu-throttle-percentage': 'int',
'*error-desc': 'str',
'*postcopy-blocktime' : 'uint32',
- '*postcopy-vcpu-blocktime': ['uint32']} }
+ '*postcopy-vcpu-blocktime': ['uint32'],
+ '*compression': 'CompressionStats'} }
##
# @query-migrate:
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
