[PATCH v2 13/14] perf kvm: Add TUI mode for stat report

Leo Yan posted 14 patches 2 years, 6 months ago
There is a newer version of this series
[PATCH v2 13/14] perf kvm: Add TUI mode for stat report
Posted by Leo Yan 2 years, 6 months ago
Since we have supported histograms list and prepared the dimensions in
the tool, this patch adds TUI mode for stat report.  It also adds UI
progress for sorting for better user experience.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/builtin-kvm.c   | 110 ++++++++++++++++++++++++++++++++++++-
 tools/perf/util/kvm-stat.h |   1 +
 2 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 5b1b2042dfed..b0be59577779 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -23,6 +23,8 @@
 #include "util/data.h"
 #include "util/ordered-events.h"
 #include "util/kvm-stat.h"
+#include "ui/browsers/hists.h"
+#include "ui/progress.h"
 #include "ui/ui.h"
 #include "util/string2.h"
 
@@ -506,10 +508,98 @@ static int kvm_hists__init(struct perf_kvm_stat *kvm)
 
 	__hists__init(&kvm_hists.hists, &kvm_hists.list);
 	perf_hpp_list__init(&kvm_hists.list);
+	kvm_hists.list.nr_header_lines = 1;
 	return kvm_hpp_list__parse(&kvm_hists.list, output_columns,
 				   kvm->sort_key);
 }
 
+static int kvm_browser__title(struct hist_browser *browser,
+			      char *buf, size_t size)
+{
+	scnprintf(buf, size, "KVM event statistics (%lu entries)",
+		  browser->nr_non_filtered_entries);
+	return 0;
+}
+
+static struct hist_browser*
+perf_kvm_browser__new(struct hists *hists)
+{
+	struct hist_browser *browser = hist_browser__new(hists);
+
+	if (browser)
+		browser->title = kvm_browser__title;
+
+	return browser;
+}
+
+static void print_result(struct perf_kvm_stat *kvm);
+
+#ifdef HAVE_SLANG_SUPPORT
+static void kvm_browser__update_nr_entries(struct hist_browser *hb)
+{
+	struct rb_node *nd = rb_first_cached(&hb->hists->entries);
+	u64 nr_entries = 0;
+
+	for (; nd; nd = rb_next(nd)) {
+		struct hist_entry *he = rb_entry(nd, struct hist_entry,
+						 rb_node);
+
+		if (!he->filtered)
+			nr_entries++;
+	}
+
+	hb->nr_non_filtered_entries = nr_entries;
+}
+
+static int kvm__hists_browse(struct hists *hists)
+{
+	struct hist_browser *browser;
+	int key = -1;
+
+	browser = perf_kvm_browser__new(hists);
+	if (browser == NULL)
+		return -1;
+
+	/* reset abort key so that it can get Ctrl-C as a key */
+	SLang_reset_tty();
+	SLang_init_tty(0, 0, 0);
+
+	kvm_browser__update_nr_entries(browser);
+
+	while (1) {
+		key = hist_browser__run(browser, "? - help", true, 0);
+
+		switch (key) {
+		case 'q':
+			goto out;
+		default:
+			break;
+		}
+	}
+
+out:
+	hist_browser__delete(browser);
+	return 0;
+}
+
+static void kvm_display(struct perf_kvm_stat *kvm)
+{
+	if (!use_browser)
+		print_result(kvm);
+	else
+		kvm__hists_browse(&kvm_hists.hists);
+}
+
+#else
+
+static void kvm_display(struct perf_kvm_stat *kvm)
+{
+	use_browser = 0;
+	print_result(kvm);
+}
+
+#endif /* HAVE_SLANG_SUPPORT */
+
 static const char *get_filename_for_perf_kvm(void)
 {
 	const char *filename;
@@ -988,8 +1078,12 @@ static int filter_cb(struct hist_entry *he, void *arg __maybe_unused)
 
 static void sort_result(void)
 {
+	struct ui_progress prog;
+
+	ui_progress__init(&prog, kvm_hists.hists.nr_entries, "Sorting...");
 	hists__collapse_resort(&kvm_hists.hists, NULL);
 	hists__output_resort_cb(&kvm_hists.hists, NULL, filter_cb);
+	ui_progress__finish();
 }
 
 static void print_vcpu_info(struct perf_kvm_stat *kvm)
@@ -1587,7 +1681,14 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
 	if (!register_kvm_events_ops(kvm))
 		goto exit;
 
-	setup_pager();
+	if (kvm->use_stdio) {
+		use_browser = 0;
+		setup_pager();
+	} else {
+		use_browser = 1;
+	}
+
+	setup_browser(false);
 
 	kvm_hists__init(kvm);
 
@@ -1596,7 +1697,7 @@ static int kvm_events_report_vcpu(struct perf_kvm_stat *kvm)
 		goto exit;
 
 	sort_result();
-	print_result(kvm);
+	kvm_display(kvm);
 
 exit:
 	return ret;
@@ -1703,6 +1804,7 @@ kvm_events_report(struct perf_kvm_stat *kvm, int argc, const char **argv)
 		OPT_STRING('p', "pid", &kvm->opts.target.pid, "pid",
 			   "analyze events only for given process id(s)"),
 		OPT_BOOLEAN('f', "force", &kvm->force, "don't complain, do it"),
+		OPT_BOOLEAN(0, "stdio", &kvm->use_stdio, "use the stdio interface"),
 		OPT_END()
 	};
 
@@ -1720,6 +1822,10 @@ kvm_events_report(struct perf_kvm_stat *kvm, int argc, const char **argv)
 					   kvm_events_report_options);
 	}
 
+#ifndef HAVE_SLANG_SUPPORT
+	kvm.use_stdio = true;
+#endif
+
 	if (!kvm->opts.target.pid)
 		kvm->opts.target.system_wide = true;
 
diff --git a/tools/perf/util/kvm-stat.h b/tools/perf/util/kvm-stat.h
index a8e919ca59f4..b66eb4e8c547 100644
--- a/tools/perf/util/kvm-stat.h
+++ b/tools/perf/util/kvm-stat.h
@@ -97,6 +97,7 @@ struct perf_kvm_stat {
 	unsigned int display_time;
 	bool live;
 	bool force;
+	bool use_stdio;
 };
 
 struct kvm_reg_events_ops {
-- 
2.34.1
Re: [PATCH v2 13/14] perf kvm: Add TUI mode for stat report
Posted by James Clark 2 years, 6 months ago

On 28/02/2023 08:41, Leo Yan wrote:
> Since we have supported histograms list and prepared the dimensions in
> the tool, this patch adds TUI mode for stat report.  It also adds UI
> progress for sorting for better user experience.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  tools/perf/builtin-kvm.c   | 110 ++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/kvm-stat.h |   1 +
>  2 files changed, 109 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 5b1b2042dfed..b0be59577779 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -23,6 +23,8 @@
>  #include "util/data.h"
>  #include "util/ordered-events.h"
>  #include "util/kvm-stat.h"
> +#include "ui/browsers/hists.h"
> +#include "ui/progress.h"
>  #include "ui/ui.h"
>  #include "util/string2.h"
>  
> @@ -506,10 +508,98 @@ static int kvm_hists__init(struct perf_kvm_stat *kvm)
>  
>  	__hists__init(&kvm_hists.hists, &kvm_hists.list);
>  	perf_hpp_list__init(&kvm_hists.list);
> +	kvm_hists.list.nr_header_lines = 1;
>  	return kvm_hpp_list__parse(&kvm_hists.list, output_columns,
>  				   kvm->sort_key);
>  }
>  
> +static int kvm_browser__title(struct hist_browser *browser,
> +			      char *buf, size_t size)
> +{
> +	scnprintf(buf, size, "KVM event statistics (%lu entries)",
> +		  browser->nr_non_filtered_entries);
> +	return 0;
> +}
> +
> +static struct hist_browser*
> +perf_kvm_browser__new(struct hists *hists)
> +{
> +	struct hist_browser *browser = hist_browser__new(hists);
> +
> +	if (browser)
> +		browser->title = kvm_browser__title;
> +
> +	return browser;
> +}
> +
> +static void print_result(struct perf_kvm_stat *kvm);
> +
> +#ifdef HAVE_SLANG_SUPPORT

Hi Leo,

I get an error because of this addition when building on x86. I think
it's because I don't have HAVE_SLANG_SUPPORT. It might be the same error
that you mentioned on v1?

   builtin-kvm.c:535:13: error: ‘print_result’ used but never defined
[-Werror]
   535 | static void print_result(struct perf_kvm_stat *kvm);

Other than that, for the whole set:

Reviewed-by: James Clark <james.clark@arm.com>

Re: [PATCH v2 13/14] perf kvm: Add TUI mode for stat report
Posted by Leo Yan 2 years, 6 months ago
Hi James,

On Tue, Feb 28, 2023 at 11:12:17AM +0000, James Clark wrote:

[...]

> > +static void print_result(struct perf_kvm_stat *kvm);
> > +
> > +#ifdef HAVE_SLANG_SUPPORT
> 
> Hi Leo,
> 
> I get an error because of this addition when building on x86. I think
> it's because I don't have HAVE_SLANG_SUPPORT. It might be the same error
> that you mentioned on v1?
> 
>    builtin-kvm.c:535:13: error: ‘print_result’ used but never defined
> [-Werror]
>    535 | static void print_result(struct perf_kvm_stat *kvm);

It's a bit weird for reporting this issue, since you could see I have
moved the function definition for print_result() ahead the macro
condition '#ifdef HAVE_SLANG_SUPPORT'.

But I found building failure with below command:

  $ make VF=1 DEBUG=1 NO_SLANG=1

Let me repin this patch for the building failures; sorry I have been
negligent.

> Other than that, for the whole set:
> 
> Reviewed-by: James Clark <james.clark@arm.com>

Thanks a lot for reviewing!  I will apply your tag in next spin
except patch 13.

Leo