All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 8/9] perf annotate browser: Support the toggle number of samples with a 'e' hotkey
@ 2017-07-13 17:46 Taeung Song
  2017-07-18 16:18 ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Taeung Song @ 2017-07-13 17:46 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Namhyung Kim, Milian Wolff, Jiri Olsa

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Milian Wolff <milian.wolff@kdab.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/ui/browsers/annotate.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 34b3189..19173b1 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -41,6 +41,7 @@ static struct annotate_browser_opt {
 	     jump_arrows,
 	     show_linenr,
 	     show_nr_jumps,
+	     show_nr_samples,
 	     show_total_period;
 } annotate_browser__opts = {
 	.use_offset	= true,
@@ -156,6 +157,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 			if (annotate_browser__opts.show_total_period) {
 				ui_browser__printf(browser, "%10" PRIu64 " ",
 						   bdl->samples[i].sample.period);
+			} else if (annotate_browser__opts.show_nr_samples) {
+				ui_browser__printf(browser, "%6" PRIu64 " ",
+						   bdl->samples[i].sample.nr_samples);
 			} else {
 				ui_browser__printf(browser, "%6.2f ",
 						   bdl->samples[i].percent);
@@ -169,6 +173,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 		else  {
 			if (annotate_browser__opts.show_total_period)
 				ui_browser__printf(browser, "%*s", 11, "Event count");
+			else if (annotate_browser__opts.show_nr_samples)
+				ui_browser__printf(browser, "%*s", 7, "Samples");
 			else
 				ui_browser__printf(browser, "%*s", 7, "Percent");
 		}
@@ -834,6 +840,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 		"o             Toggle disassembler output/simplified view\n"
 		"s             Toggle source code view\n"
 		"t             Toggle total period view\n"
+		"e             Toggle number of samples\n"
 		"/             Search string\n"
 		"k             Toggle line numbers\n"
 		"r             Run available scripts\n"
@@ -914,6 +921,12 @@ static int annotate_browser__run(struct annotate_browser *browser,
 			  !annotate_browser__opts.show_total_period;
 			annotate_browser__update_addr_width(browser);
 			continue;
+		case 'e':
+			annotate_browser__opts.show_total_period = false;
+			annotate_browser__opts.show_nr_samples =
+				!annotate_browser__opts.show_nr_samples;
+			annotate_browser__update_addr_width(browser);
+			continue;
 		case K_LEFT:
 		case K_ESC:
 		case 'q':
@@ -934,9 +947,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
 int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 			     struct hist_browser_timer *hbt)
 {
-	/* Set default value for show_total_period.  */
+	/* Set default value for show_total_period and show_nr_samples  */
 	annotate_browser__opts.show_total_period =
 	  symbol_conf.show_total_period;
+	annotate_browser__opts.show_nr_samples =
+		symbol_conf.show_nr_samples;
 
 	return symbol__tui_annotate(ms->sym, ms->map, evsel, hbt);
 }
@@ -1187,6 +1202,7 @@ static struct annotate_config {
 	ANNOTATE_CFG(jump_arrows),
 	ANNOTATE_CFG(show_linenr),
 	ANNOTATE_CFG(show_nr_jumps),
+	ANNOTATE_CFG(show_nr_samples),
 	ANNOTATE_CFG(show_total_period),
 	ANNOTATE_CFG(use_offset),
 };
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 8/9] perf annotate browser: Support the toggle number of samples with a 'e' hotkey
  2017-07-13 17:46 [PATCH v2 8/9] perf annotate browser: Support the toggle number of samples with a 'e' hotkey Taeung Song
@ 2017-07-18 16:18 ` Namhyung Kim
  2017-07-19 17:15   ` Taeung Song
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2017-07-18 16:18 UTC (permalink / raw)
  To: Taeung Song
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Milian Wolff, Jiri Olsa,
	kernel-team

On Fri, Jul 14, 2017 at 02:46:16AM +0900, Taeung Song wrote:
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Milian Wolff <milian.wolff@kdab.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>

Hmm.. IIUC there're 3 modes of annotation view: percent, period and
sample, right?  The existing 't' hotkey seems to toggle between
percent and period.  And you added 'e' for sample and percent, right?

I'm not sure percent by sample and percent by period are both needed.
If so, I think it's better to add a hotkey to toggle between percent
and value (both for period and sample).  And existing key should
toggle between sample and period.

If percent by sample is not meaningful, I'd rather make the hotkey to
circulate through percent, period and sample.

Thanks,
Namhyung


> ---
>  tools/perf/ui/browsers/annotate.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 34b3189..19173b1 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -41,6 +41,7 @@ static struct annotate_browser_opt {
>  	     jump_arrows,
>  	     show_linenr,
>  	     show_nr_jumps,
> +	     show_nr_samples,
>  	     show_total_period;
>  } annotate_browser__opts = {
>  	.use_offset	= true,
> @@ -156,6 +157,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>  			if (annotate_browser__opts.show_total_period) {
>  				ui_browser__printf(browser, "%10" PRIu64 " ",
>  						   bdl->samples[i].sample.period);
> +			} else if (annotate_browser__opts.show_nr_samples) {
> +				ui_browser__printf(browser, "%6" PRIu64 " ",
> +						   bdl->samples[i].sample.nr_samples);
>  			} else {
>  				ui_browser__printf(browser, "%6.2f ",
>  						   bdl->samples[i].percent);
> @@ -169,6 +173,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>  		else  {
>  			if (annotate_browser__opts.show_total_period)
>  				ui_browser__printf(browser, "%*s", 11, "Event count");
> +			else if (annotate_browser__opts.show_nr_samples)
> +				ui_browser__printf(browser, "%*s", 7, "Samples");
>  			else
>  				ui_browser__printf(browser, "%*s", 7, "Percent");
>  		}
> @@ -834,6 +840,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  		"o             Toggle disassembler output/simplified view\n"
>  		"s             Toggle source code view\n"
>  		"t             Toggle total period view\n"
> +		"e             Toggle number of samples\n"
>  		"/             Search string\n"
>  		"k             Toggle line numbers\n"
>  		"r             Run available scripts\n"
> @@ -914,6 +921,12 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  			  !annotate_browser__opts.show_total_period;
>  			annotate_browser__update_addr_width(browser);
>  			continue;
> +		case 'e':
> +			annotate_browser__opts.show_total_period = false;
> +			annotate_browser__opts.show_nr_samples =
> +				!annotate_browser__opts.show_nr_samples;
> +			annotate_browser__update_addr_width(browser);
> +			continue;
>  		case K_LEFT:
>  		case K_ESC:
>  		case 'q':
> @@ -934,9 +947,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
>  			     struct hist_browser_timer *hbt)
>  {
> -	/* Set default value for show_total_period.  */
> +	/* Set default value for show_total_period and show_nr_samples  */
>  	annotate_browser__opts.show_total_period =
>  	  symbol_conf.show_total_period;
> +	annotate_browser__opts.show_nr_samples =
> +		symbol_conf.show_nr_samples;
>  
>  	return symbol__tui_annotate(ms->sym, ms->map, evsel, hbt);
>  }
> @@ -1187,6 +1202,7 @@ static struct annotate_config {
>  	ANNOTATE_CFG(jump_arrows),
>  	ANNOTATE_CFG(show_linenr),
>  	ANNOTATE_CFG(show_nr_jumps),
> +	ANNOTATE_CFG(show_nr_samples),
>  	ANNOTATE_CFG(show_total_period),
>  	ANNOTATE_CFG(use_offset),
>  };
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 8/9] perf annotate browser: Support the toggle number of samples with a 'e' hotkey
  2017-07-18 16:18 ` Namhyung Kim
@ 2017-07-19 17:15   ` Taeung Song
  0 siblings, 0 replies; 3+ messages in thread
From: Taeung Song @ 2017-07-19 17:15 UTC (permalink / raw)
  To: Namhyung Kim, Arnaldo Carvalho de Melo
  Cc: linux-kernel, Milian Wolff, Jiri Olsa, kernel-team



On 07/19/2017 01:18 AM, Namhyung Kim wrote:
> On Fri, Jul 14, 2017 at 02:46:16AM +0900, Taeung Song wrote:
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Milian Wolff <milian.wolff@kdab.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> 
> Hmm.. IIUC there're 3 modes of annotation view: percent, period and
> sample, right?  The existing 't' hotkey seems to toggle between
> percent and period.  And you added 'e' for sample and percent, right?
> 
> I'm not sure percent by sample and percent by period are both needed.
> If so, I think it's better to add a hotkey to toggle between percent
> and value (both for period and sample).  And existing key should
> toggle between sample and period.
> 
> If percent by sample is not meaningful, I'd rather make the hotkey to
> circulate through percent, period and sample.
> 
> Thanks,
> Namhyung
> 
> 

I agree on it. It seems to be better than two hot key !
I'll change this patch based on your comment.

Arnaldo, what do you think about this ?

   On annotate TUI browser
   hot key: 't'
   circulating: "Percent" -> "Sample" -> "Period" -> "Percent" -> ...


Thanks,
Taeung

>> ---
>>   tools/perf/ui/browsers/annotate.c | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
>> index 34b3189..19173b1 100644
>> --- a/tools/perf/ui/browsers/annotate.c
>> +++ b/tools/perf/ui/browsers/annotate.c
>> @@ -41,6 +41,7 @@ static struct annotate_browser_opt {
>>   	     jump_arrows,
>>   	     show_linenr,
>>   	     show_nr_jumps,
>> +	     show_nr_samples,
>>   	     show_total_period;
>>   } annotate_browser__opts = {
>>   	.use_offset	= true,
>> @@ -156,6 +157,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>>   			if (annotate_browser__opts.show_total_period) {
>>   				ui_browser__printf(browser, "%10" PRIu64 " ",
>>   						   bdl->samples[i].sample.period);
>> +			} else if (annotate_browser__opts.show_nr_samples) {
>> +				ui_browser__printf(browser, "%6" PRIu64 " ",
>> +						   bdl->samples[i].sample.nr_samples);
>>   			} else {
>>   				ui_browser__printf(browser, "%6.2f ",
>>   						   bdl->samples[i].percent);
>> @@ -169,6 +173,8 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>>   		else  {
>>   			if (annotate_browser__opts.show_total_period)
>>   				ui_browser__printf(browser, "%*s", 11, "Event count");
>> +			else if (annotate_browser__opts.show_nr_samples)
>> +				ui_browser__printf(browser, "%*s", 7, "Samples");
>>   			else
>>   				ui_browser__printf(browser, "%*s", 7, "Percent");
>>   		}
>> @@ -834,6 +840,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>>   		"o             Toggle disassembler output/simplified view\n"
>>   		"s             Toggle source code view\n"
>>   		"t             Toggle total period view\n"
>> +		"e             Toggle number of samples\n"
>>   		"/             Search string\n"
>>   		"k             Toggle line numbers\n"
>>   		"r             Run available scripts\n"
>> @@ -914,6 +921,12 @@ static int annotate_browser__run(struct annotate_browser *browser,
>>   			  !annotate_browser__opts.show_total_period;
>>   			annotate_browser__update_addr_width(browser);
>>   			continue;
>> +		case 'e':
>> +			annotate_browser__opts.show_total_period = false;
>> +			annotate_browser__opts.show_nr_samples =
>> +				!annotate_browser__opts.show_nr_samples;
>> +			annotate_browser__update_addr_width(browser);
>> +			continue;
>>   		case K_LEFT:
>>   		case K_ESC:
>>   		case 'q':
>> @@ -934,9 +947,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
>>   int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
>>   			     struct hist_browser_timer *hbt)
>>   {
>> -	/* Set default value for show_total_period.  */
>> +	/* Set default value for show_total_period and show_nr_samples  */
>>   	annotate_browser__opts.show_total_period =
>>   	  symbol_conf.show_total_period;
>> +	annotate_browser__opts.show_nr_samples =
>> +		symbol_conf.show_nr_samples;
>>   
>>   	return symbol__tui_annotate(ms->sym, ms->map, evsel, hbt);
>>   }
>> @@ -1187,6 +1202,7 @@ static struct annotate_config {
>>   	ANNOTATE_CFG(jump_arrows),
>>   	ANNOTATE_CFG(show_linenr),
>>   	ANNOTATE_CFG(show_nr_jumps),
>> +	ANNOTATE_CFG(show_nr_samples),
>>   	ANNOTATE_CFG(show_total_period),
>>   	ANNOTATE_CFG(use_offset),
>>   };
>> -- 
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-07-19 17:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-13 17:46 [PATCH v2 8/9] perf annotate browser: Support the toggle number of samples with a 'e' hotkey Taeung Song
2017-07-18 16:18 ` Namhyung Kim
2017-07-19 17:15   ` Taeung Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.