From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Ingo Molnar <mingo@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Clark Williams <williams@redhat.com>,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Andi Kleen <ak@linux.intel.com>,
Jin Yao <yao.jin@linux.intel.com>,
Kan Liang <kan.liang@intel.com>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH 07/12] perf hists browser: Allow passing an initial hotkey
Date: Thu, 19 Dec 2019 14:26:42 -0300 [thread overview]
Message-ID: <20191219172642.GB13699@kernel.org> (raw)
In-Reply-To: <20191218142321.GB15571@krava>
Em Wed, Dec 18, 2019 at 03:23:21PM +0100, Jiri Olsa escreveu:
> On Wed, Dec 18, 2019 at 11:08:31AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Dec 18, 2019 at 09:08:18AM +0100, Jiri Olsa escreveu:
> > > On Tue, Dec 17, 2019 at 11:48:23AM -0300, Arnaldo Carvalho de Melo wrote:
> > > > + if (key)
> > > > + goto do_hotkey;
> > > > +
> > > > while (1) {
> > > > key = ui_browser__run(&browser->b, delay_secs);
> > > > -
> > > > +do_hotkey:
> > > or we could switch the 'swtich' and ui_browser__run, and get rid of the goto, like:
> > > while (1) {
> > > switch (key) {
> > > ...
> > > }
> > >
> > > key = ui_browser__run(&browser->b, delay_secs);
> > > }
> > I think those are equivalent and having the test like I did is more
> > clear, i.e. "has this key been provided" instead of going to the switch
> > just to hit the default case for the zero in key and call
> > ui_browser__run().
> sure, I just don't like goto other than for error handling,
> looks too hacky to me ;-) but of course it's your call
How about the one below?
- Arnaldo
commit 9290297c911ad6c315c5f46f35afa3ce517a4c9f
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Thu Dec 12 15:31:40 2019 -0300
perf hists browser: Allow passing an initial hotkey
Sometimes we're in an outer code, like the main hists browser popup menu
and the user follows a suggestion about using some hotkey, and that
hotkey is really handled by hists_browser__run(), so allow for calling
it with that hotkey, making it handle it instead of waiting for the user
to press one.
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lkml.kernel.org/n/tip-xv2l7i6o4urn37nv1h40ryfs@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index e69f44941aad..346351260c0b 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -2384,7 +2384,7 @@ static int perf_c2c__browse_cacheline(struct hist_entry *he)
c2c_browser__update_nr_entries(browser);
while (1) {
- key = hist_browser__run(browser, "? - help", true);
+ key = hist_browser__run(browser, "? - help", true, 0);
switch (key) {
case 's':
@@ -2453,7 +2453,7 @@ static int perf_c2c__hists_browse(struct hists *hists)
c2c_browser__update_nr_entries(browser);
while (1) {
- key = hist_browser__run(browser, "? - help", true);
+ key = hist_browser__run(browser, "? - help", true, 0);
switch (key) {
case 'q':
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 6dfdd8d5a743..ac118aef5ed1 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -672,10 +672,81 @@ static int hist_browser__title(struct hist_browser *browser, char *bf, size_t si
return browser->title ? browser->title(browser, bf, size) : 0;
}
+static int hist_browser__handle_hotkey(struct hist_browser *browser, bool warn_lost_event, char *title, int key)
+{
+ switch (key) {
+ case K_TIMER: {
+ struct hist_browser_timer *hbt = browser->hbt;
+ u64 nr_entries;
+
+ WARN_ON_ONCE(!hbt);
+
+ if (hbt)
+ hbt->timer(hbt->arg);
+
+ if (hist_browser__has_filter(browser) || symbol_conf.report_hierarchy)
+ hist_browser__update_nr_entries(browser);
+
+ nr_entries = hist_browser__nr_entries(browser);
+ ui_browser__update_nr_entries(&browser->b, nr_entries);
+
+ if (warn_lost_event &&
+ (browser->hists->stats.nr_lost_warned !=
+ browser->hists->stats.nr_events[PERF_RECORD_LOST])) {
+ browser->hists->stats.nr_lost_warned =
+ browser->hists->stats.nr_events[PERF_RECORD_LOST];
+ ui_browser__warn_lost_events(&browser->b);
+ }
+
+ hist_browser__title(browser, title, sizeof(title));
+ ui_browser__show_title(&browser->b, title);
+ break;
+ }
+ case 'D': { /* Debug */
+ struct hist_entry *h = rb_entry(browser->b.top, struct hist_entry, rb_node);
+ static int seq;
+
+ ui_helpline__pop();
+ ui_helpline__fpush("%d: nr_ent=(%d,%d), etl: %d, rows=%d, idx=%d, fve: idx=%d, row_off=%d, nrows=%d",
+ seq++, browser->b.nr_entries, browser->hists->nr_entries,
+ browser->b.extra_title_lines, browser->b.rows,
+ browser->b.index, browser->b.top_idx, h->row_offset, h->nr_rows);
+ }
+ break;
+ case 'C':
+ /* Collapse the whole world. */
+ hist_browser__set_folding(browser, false);
+ break;
+ case 'c':
+ /* Collapse the selected entry. */
+ hist_browser__set_folding_selected(browser, false);
+ break;
+ case 'E':
+ /* Expand the whole world. */
+ hist_browser__set_folding(browser, true);
+ break;
+ case 'e':
+ /* Expand the selected entry. */
+ hist_browser__set_folding_selected(browser, true);
+ break;
+ case 'H':
+ browser->show_headers = !browser->show_headers;
+ hist_browser__update_rows(browser);
+ break;
+ case '+':
+ if (hist_browser__toggle_fold(browser))
+ break;
+ /* fall thru */
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
int hist_browser__run(struct hist_browser *browser, const char *help,
- bool warn_lost_event)
+ bool warn_lost_event, int key)
{
- int key;
char title[160];
struct hist_browser_timer *hbt = browser->hbt;
int delay_secs = hbt ? hbt->refresh : 0;
@@ -688,79 +759,14 @@ int hist_browser__run(struct hist_browser *browser, const char *help,
if (ui_browser__show(&browser->b, title, "%s", help) < 0)
return -1;
+ if (key && hist_browser__handle_hotkey(browser, warn_lost_event, title, key))
+ goto out;
+
while (1) {
key = ui_browser__run(&browser->b, delay_secs);
- switch (key) {
- case K_TIMER: {
- u64 nr_entries;
-
- WARN_ON_ONCE(!hbt);
-
- if (hbt)
- hbt->timer(hbt->arg);
-
- if (hist_browser__has_filter(browser) ||
- symbol_conf.report_hierarchy)
- hist_browser__update_nr_entries(browser);
-
- nr_entries = hist_browser__nr_entries(browser);
- ui_browser__update_nr_entries(&browser->b, nr_entries);
-
- if (warn_lost_event &&
- (browser->hists->stats.nr_lost_warned !=
- browser->hists->stats.nr_events[PERF_RECORD_LOST])) {
- browser->hists->stats.nr_lost_warned =
- browser->hists->stats.nr_events[PERF_RECORD_LOST];
- ui_browser__warn_lost_events(&browser->b);
- }
-
- hist_browser__title(browser, title, sizeof(title));
- ui_browser__show_title(&browser->b, title);
- continue;
- }
- case 'D': { /* Debug */
- static int seq;
- struct hist_entry *h = rb_entry(browser->b.top,
- struct hist_entry, rb_node);
- ui_helpline__pop();
- ui_helpline__fpush("%d: nr_ent=(%d,%d), etl: %d, rows=%d, idx=%d, fve: idx=%d, row_off=%d, nrows=%d",
- seq++, browser->b.nr_entries,
- browser->hists->nr_entries,
- browser->b.extra_title_lines,
- browser->b.rows,
- browser->b.index,
- browser->b.top_idx,
- h->row_offset, h->nr_rows);
- }
- break;
- case 'C':
- /* Collapse the whole world. */
- hist_browser__set_folding(browser, false);
- break;
- case 'c':
- /* Collapse the selected entry. */
- hist_browser__set_folding_selected(browser, false);
- break;
- case 'E':
- /* Expand the whole world. */
- hist_browser__set_folding(browser, true);
+ if (hist_browser__handle_hotkey(browser, warn_lost_event, title, key))
break;
- case 'e':
- /* Expand the selected entry. */
- hist_browser__set_folding_selected(browser, true);
- break;
- case 'H':
- browser->show_headers = !browser->show_headers;
- hist_browser__update_rows(browser);
- break;
- case '+':
- if (hist_browser__toggle_fold(browser))
- break;
- /* fall thru */
- default:
- goto out;
- }
}
out:
ui_browser__hide(&browser->b);
@@ -2994,8 +3000,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
nr_options = 0;
- key = hist_browser__run(browser, helpline,
- warn_lost_event);
+ key = hist_browser__run(browser, helpline, warn_lost_event, 0);
if (browser->he_selection != NULL) {
thread = hist_browser__selected_thread(browser);
@@ -3573,7 +3578,7 @@ int block_hists_tui_browse(struct block_hist *bh, struct evsel *evsel,
memset(&action, 0, sizeof(action));
while (1) {
- key = hist_browser__run(browser, "? - help", true);
+ key = hist_browser__run(browser, "? - help", true, 0);
switch (key) {
case 'q':
diff --git a/tools/perf/ui/browsers/hists.h b/tools/perf/ui/browsers/hists.h
index 078f2f2c7abd..1e938d9ffa5e 100644
--- a/tools/perf/ui/browsers/hists.h
+++ b/tools/perf/ui/browsers/hists.h
@@ -34,7 +34,7 @@ struct hist_browser {
struct hist_browser *hist_browser__new(struct hists *hists);
void hist_browser__delete(struct hist_browser *browser);
int hist_browser__run(struct hist_browser *browser, const char *help,
- bool warn_lost_event);
+ bool warn_lost_event, int key);
void hist_browser__init(struct hist_browser *browser,
struct hists *hists);
#endif /* _PERF_UI_BROWSER_HISTS_H_ */
next prev parent reply other threads:[~2019-12-19 17:26 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-17 14:48 [RFC] perf report/top with callchain fixes improvements Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 01/12] perf hists browser: Restore ESC as "Zoom out" of DSO/thread/etc Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 02/12] perf report/top: Make ENTER consistently bring up menu Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 03/12] perf report/top: Add menu entry for toggling callchain expansion Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 04/12] perf report/top: Improve toggle callchain menu option Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 05/12] perf hists browser: Generalize the do_zoom_dso() function Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 06/12] perf report/top: Add 'k' hotkey to zoom directly into the kernel map Arnaldo Carvalho de Melo
2019-12-20 6:48 ` Namhyung Kim
2019-12-20 12:17 ` Arnaldo Carvalho de Melo
2019-12-21 7:20 ` Namhyung Kim
2019-12-17 14:48 ` [PATCH 07/12] perf hists browser: Allow passing an initial hotkey Arnaldo Carvalho de Melo
2019-12-18 8:08 ` Jiri Olsa
2019-12-18 14:08 ` Arnaldo Carvalho de Melo
2019-12-18 14:23 ` Jiri Olsa
2019-12-19 17:26 ` Arnaldo Carvalho de Melo [this message]
2019-12-19 21:44 ` Jiri Olsa
2019-12-17 14:48 ` [PATCH 08/12] tools ui popup: Allow returning hotkeys Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 09/12] perf report/top: Allow pressing hotkeys in the options popup menu Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 10/12] perf report/top: Do not offer annotation for symbols without samples Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 11/12] perf report/top: Make 'e' visible in the help and make it toggle showing callchains Arnaldo Carvalho de Melo
2019-12-17 14:48 ` [PATCH 12/12] perf maps: Set maps pointer in the kmap area for kernel maps Arnaldo Carvalho de Melo
2019-12-18 9:05 ` Jiri Olsa
2019-12-18 14:24 ` Arnaldo Carvalho de Melo
2019-12-18 18:22 ` Arnaldo Carvalho de Melo
2019-12-18 19:01 ` Arnaldo Carvalho de Melo
2019-12-19 9:24 ` Jiri Olsa
2019-12-18 9:07 ` Jiri Olsa
2019-12-18 14:20 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191219172642.GB13699@kernel.org \
--to=arnaldo.melo@gmail.com \
--cc=acme@redhat.com \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=jolsa@redhat.com \
--cc=kan.liang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=williams@redhat.com \
--cc=yao.jin@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.