* [GIT PULL 0/1] perf/core ui browser improvement @ 2010-09-13 16:34 Arnaldo Carvalho de Melo 2010-09-13 16:34 ` [PATCH 1/1] perf ui browser: Don't use windows, slang is enough Arnaldo Carvalho de Melo 2010-09-13 17:48 ` [GIT PULL 0/1] perf/core ui browser improvement Ingo Molnar 0 siblings, 2 replies; 9+ messages in thread From: Arnaldo Carvalho de Melo @ 2010-09-13 16:34 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Arnaldo Carvalho de Melo, Christoph Hellwig, Frederic Weisbecker, Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net> Hi Ingo, Please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core Screenshot showing how it looks like now: http://vger.kernel.org/~acme/perf/perf-report-tui-no-windows.png Regards, - Arnaldo Arnaldo Carvalho de Melo (1): perf ui browser: Don't use windows, slang is enough tools/perf/util/ui/browser.c | 24 +++++++++----------- tools/perf/util/ui/browsers/hists.c | 41 ++++++++++++++++------------------ tools/perf/util/ui/browsers/map.c | 9 +++---- 3 files changed, 34 insertions(+), 40 deletions(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/1] perf ui browser: Don't use windows, slang is enough 2010-09-13 16:34 [GIT PULL 0/1] perf/core ui browser improvement Arnaldo Carvalho de Melo @ 2010-09-13 16:34 ` Arnaldo Carvalho de Melo 2010-09-13 17:48 ` [GIT PULL 0/1] perf/core ui browser improvement Ingo Molnar 1 sibling, 0 replies; 9+ messages in thread From: Arnaldo Carvalho de Melo @ 2010-09-13 16:34 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Arnaldo Carvalho de Melo, Christoph Hellwig, Frederic Weisbecker, Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi From: Arnaldo Carvalho de Melo <acme@redhat.com> They are useless and take away precious columns and lines, so stop using windows. One more step in removing newt code, that after all is not being useful at all for the coalescing TUI model in perf. Suggested-by: Christoph Hellwig <hch@infradead.org> Cc: Christoph Hellwig <hch@infradead.org> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Ingo Molnar <mingo@elte.hu> Cc: Mike Galbraith <efault@gmx.de> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: Tom Zanussi <tzanussi@gmail.com> LKML-Reference: <20100822082003.GB7365@infradead.org> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/ui/browser.c | 24 +++++++++----------- tools/perf/util/ui/browsers/hists.c | 41 ++++++++++++++++------------------ tools/perf/util/ui/browsers/map.c | 9 +++---- 3 files changed, 34 insertions(+), 40 deletions(-) diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c index 930c4ac..6d0df80 100644 --- a/tools/perf/util/ui/browser.c +++ b/tools/perf/util/ui/browser.c @@ -130,13 +130,10 @@ void ui_browser__refresh_dimensions(struct ui_browser *self) int cols, rows; newtGetScreenSize(&cols, &rows); - if (self->width > cols - 4) - self->width = cols - 4; - self->height = rows - 5; - if (self->height > self->nr_entries) - self->height = self->nr_entries; - self->y = (rows - self->height) / 2; - self->x = (cols - self->width) / 2; + self->width = cols - 1; + self->height = rows - 2; + self->y = 1; + self->x = 0; } void ui_browser__reset_index(struct ui_browser *self) @@ -168,22 +165,24 @@ int ui_browser__show(struct ui_browser *self, const char *title, NEWT_KEY_PGDN, NEWT_KEY_HOME, NEWT_KEY_END, ' ', NEWT_KEY_LEFT, NEWT_KEY_ESCAPE, 'q', CTRL('c'), 0 }; - if (self->form != NULL) { + if (self->form != NULL) newtFormDestroy(self->form); - newtPopWindow(); - } + ui_browser__refresh_dimensions(self); - newtCenteredWindow(self->width, self->height, title); self->form = newtForm(NULL, NULL, 0); if (self->form == NULL) return -1; - self->sb = newtVerticalScrollbar(self->width, 0, self->height, + self->sb = newtVerticalScrollbar(self->width, 1, self->height, HE_COLORSET_NORMAL, HE_COLORSET_SELECTED); if (self->sb == NULL) return -1; + SLsmg_gotorc(0, 0); + ui_browser__set_color(self, NEWT_COLORSET_ROOT); + slsmg_write_nstring(title, self->width); + ui_browser__add_exit_keys(self, keys); newtFormAddComponent(self->form, self->sb); @@ -196,7 +195,6 @@ int ui_browser__show(struct ui_browser *self, const char *title, void ui_browser__hide(struct ui_browser *self) { newtFormDestroy(self->form); - newtPopWindow(); self->form = NULL; ui_helpline__pop(); } diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c index 2fc1ba3..ebda8c3 100644 --- a/tools/perf/util/ui/browsers/hists.c +++ b/tools/perf/util/ui/browsers/hists.c @@ -293,19 +293,12 @@ static int hist_browser__run(struct hist_browser *self, const char *title) int key; int exit_keys[] = { 'a', '?', 'h', 'C', 'd', 'D', 'E', 't', NEWT_KEY_ENTER, NEWT_KEY_RIGHT, NEWT_KEY_LEFT, 0, }; - char str[256], unit; - unsigned long nr_events = self->hists->stats.nr_events[PERF_RECORD_SAMPLE]; self->b.entries = &self->hists->entries; self->b.nr_entries = self->hists->nr_entries; hist_browser__refresh_dimensions(self); - nr_events = convert_unit(nr_events, &unit); - snprintf(str, sizeof(str), "Events: %lu%c ", - nr_events, unit); - newtDrawRootText(0, 0, str); - if (ui_browser__show(&self->b, title, "Press '?' for help on key bindings") < 0) return -1; @@ -782,21 +775,26 @@ static struct thread *hist_browser__selected_thread(struct hist_browser *self) return self->he_selection->thread; } -static int hist_browser__title(char *bf, size_t size, const char *ev_name, - const struct dso *dso, const struct thread *thread) +static int hists__browser_title(struct hists *self, char *bf, size_t size, + const char *ev_name, const struct dso *dso, + const struct thread *thread) { - int printed = 0; + char unit; + int printed; + unsigned long nr_events = self->stats.nr_events[PERF_RECORD_SAMPLE]; + + nr_events = convert_unit(nr_events, &unit); + printed = snprintf(bf, size, "Events: %lu%c %s", nr_events, unit, ev_name); if (thread) printed += snprintf(bf + printed, size - printed, - "Thread: %s(%d)", - (thread->comm_set ? thread->comm : ""), + ", Thread: %s(%d)", + (thread->comm_set ? thread->comm : ""), thread->pid); if (dso) printed += snprintf(bf + printed, size - printed, - "%sDSO: %s", thread ? " " : "", - dso->short_name); - return printed ?: snprintf(bf, size, "Event: %s", ev_name); + ", DSO: %s", dso->short_name); + return printed; } int hists__browse(struct hists *self, const char *helpline, const char *ev_name) @@ -817,9 +815,8 @@ int hists__browse(struct hists *self, const char *helpline, const char *ev_name) ui_helpline__push(helpline); - hist_browser__title(msg, sizeof(msg), ev_name, - dso_filter, thread_filter); - + hists__browser_title(self, msg, sizeof(msg), ev_name, + dso_filter, thread_filter); while (1) { const struct thread *thread; const struct dso *dso; @@ -957,8 +954,8 @@ zoom_out_dso: pstack__push(fstack, &dso_filter); } hists__filter_by_dso(self, dso_filter); - hist_browser__title(msg, sizeof(msg), ev_name, - dso_filter, thread_filter); + hists__browser_title(self, msg, sizeof(msg), ev_name, + dso_filter, thread_filter); hist_browser__reset(browser); } else if (choice == zoom_thread) { zoom_thread: @@ -975,8 +972,8 @@ zoom_out_thread: pstack__push(fstack, &thread_filter); } hists__filter_by_thread(self, thread_filter); - hist_browser__title(msg, sizeof(msg), ev_name, - dso_filter, thread_filter); + hists__browser_title(self, msg, sizeof(msg), ev_name, + dso_filter, thread_filter); hist_browser__reset(browser); } } diff --git a/tools/perf/util/ui/browsers/map.c b/tools/perf/util/ui/browsers/map.c index 1bf0979..e35437d 100644 --- a/tools/perf/util/ui/browsers/map.c +++ b/tools/perf/util/ui/browsers/map.c @@ -46,7 +46,6 @@ out_free_form: struct map_browser { struct ui_browser b; struct map *map; - u16 namelen; u8 addrlen; }; @@ -55,13 +54,16 @@ static void map_browser__write(struct ui_browser *self, void *nd, int row) struct symbol *sym = rb_entry(nd, struct symbol, rb_node); struct map_browser *mb = container_of(self, struct map_browser, b); bool current_entry = ui_browser__is_current_entry(self, row); + int width; ui_browser__set_percent_color(self, 0, current_entry); slsmg_printf("%*llx %*llx %c ", mb->addrlen, sym->start, mb->addrlen, sym->end, sym->binding == STB_GLOBAL ? 'g' : sym->binding == STB_LOCAL ? 'l' : 'w'); - slsmg_write_nstring(sym->name, mb->namelen); + width = self->width - ((mb->addrlen * 2) + 4); + if (width > 0) + slsmg_write_nstring(sym->name, width); } /* FIXME uber-kludgy, see comment on cmd_report... */ @@ -139,8 +141,6 @@ int map__browse(struct map *self) for (nd = rb_first(mb.b.entries); nd; nd = rb_next(nd)) { struct symbol *pos = rb_entry(nd, struct symbol, rb_node); - if (mb.namelen < pos->namelen) - mb.namelen = pos->namelen; if (maxaddr < pos->end) maxaddr = pos->end; if (verbose) { @@ -151,6 +151,5 @@ int map__browse(struct map *self) } mb.addrlen = snprintf(tmp, sizeof(tmp), "%llx", maxaddr); - mb.b.width += mb.addrlen * 2 + 4 + mb.namelen; return map_browser__run(&mb); } -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [GIT PULL 0/1] perf/core ui browser improvement 2010-09-13 16:34 [GIT PULL 0/1] perf/core ui browser improvement Arnaldo Carvalho de Melo 2010-09-13 16:34 ` [PATCH 1/1] perf ui browser: Don't use windows, slang is enough Arnaldo Carvalho de Melo @ 2010-09-13 17:48 ` Ingo Molnar 2010-09-13 19:07 ` Arnaldo Carvalho de Melo 1 sibling, 1 reply; 9+ messages in thread From: Ingo Molnar @ 2010-09-13 17:48 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Arnaldo Carvalho de Melo, Christoph Hellwig, Frederic Weisbecker, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi * Arnaldo Carvalho de Melo <acme@infradead.org> wrote: > From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net> > > Hi Ingo, > > Please pull from: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/core Pulled, thanks Arnaldo! > Screenshot showing how it looks like now: > > http://vger.kernel.org/~acme/perf/perf-report-tui-no-windows.png Nice, it's more economic with screen real estate now :-) btw., a small detail: could we please rename [kernel.kallsyms] to [kernel]? The user is not really interested where the symbols came from (kallsyms or vmlinux), in 99.9% of the cases - and the many repetitive .kallsyms instances look uninformative and somewhat confusing. Thanks, Ingo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL 0/1] perf/core ui browser improvement 2010-09-13 17:48 ` [GIT PULL 0/1] perf/core ui browser improvement Ingo Molnar @ 2010-09-13 19:07 ` Arnaldo Carvalho de Melo 2010-09-13 20:42 ` Ingo Molnar 0 siblings, 1 reply; 9+ messages in thread From: Arnaldo Carvalho de Melo @ 2010-09-13 19:07 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Christoph Hellwig, Frederic Weisbecker, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi Em Mon, Sep 13, 2010 at 07:48:45PM +0200, Ingo Molnar escreveu: > Nice, it's more economic with screen real estate now :-) > > btw., a small detail: could we please rename [kernel.kallsyms] to > [kernel]? The user is not really interested where the symbols came from > (kallsyms or vmlinux), in 99.9% of the cases - and the many repetitive > .kallsyms instances look uninformative and somewhat confusing. Sure, I'll probably even remove the brackets :-) - Arnaldo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL 0/1] perf/core ui browser improvement 2010-09-13 19:07 ` Arnaldo Carvalho de Melo @ 2010-09-13 20:42 ` Ingo Molnar 2010-09-13 21:29 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 9+ messages in thread From: Ingo Molnar @ 2010-09-13 20:42 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Christoph Hellwig, Frederic Weisbecker, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi * Arnaldo Carvalho de Melo <acme@infradead.org> wrote: > Em Mon, Sep 13, 2010 at 07:48:45PM +0200, Ingo Molnar escreveu: > > Nice, it's more economic with screen real estate now :-) > > > > btw., a small detail: could we please rename [kernel.kallsyms] to > > [kernel]? The user is not really interested where the symbols came from > > (kallsyms or vmlinux), in 99.9% of the cases - and the many repetitive > > .kallsyms instances look uninformative and somewhat confusing. > > Sure, I'll probably even remove the brackets :-) The brackets kind of make sense, they signal that it's a container (which it is) - same for DSOs. Thanks, Ingo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL 0/1] perf/core ui browser improvement 2010-09-13 20:42 ` Ingo Molnar @ 2010-09-13 21:29 ` Arnaldo Carvalho de Melo 2010-09-14 7:54 ` Peter Zijlstra 0 siblings, 1 reply; 9+ messages in thread From: Arnaldo Carvalho de Melo @ 2010-09-13 21:29 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Christoph Hellwig, Frederic Weisbecker, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi Em Mon, Sep 13, 2010 at 10:42:17PM +0200, Ingo Molnar escreveu: > > Em Mon, Sep 13, 2010 at 07:48:45PM +0200, Ingo Molnar escreveu: > > > Nice, it's more economic with screen real estate now :-) > > > > > > btw., a small detail: could we please rename [kernel.kallsyms] to > > > [kernel]? The user is not really interested where the symbols came from > > > (kallsyms or vmlinux), in 99.9% of the cases - and the many repetitive > > > .kallsyms instances look uninformative and somewhat confusing. > > > > Sure, I'll probably even remove the brackets :-) > > The brackets kind of make sense, they signal that it's a container > (which it is) - same for DSOs. + 3.00% find [kernel.kallsyms] [k] n_tty_write + 2.59% find [kernel.kallsyms] [k] _raw_spin_lock_irqsave + 2.43% find find [.] knuth_morris_pratt_unibyte + 2.22% sshd sshd [.] 41841 + 2.16% swapper [kernel.kallsyms] [k] mwait_idle + 1.65% find libc-2.12.so [.] __GI_vfprintf Right now DSOs doesn't use brackets, just the kernel and modules, that differentiates them from userspace DSOs, but then there is the next column, where we have (under brackets) "k", "." or "H" for kernel, userspace, hypervisor, so we can use this info to know the context, no need for brackets around the DSO as well. Arguably I'd remove also the brackets around the context, they also don't convey useful information IMHO, and also would replace '.' with 'u', so it would become: + 3.00% find kernel k n_tty_write + 2.59% find kernel k _raw_spin_lock_irqsave + 2.43% find find u knuth_morris_pratt_unibyte + 2.22% sshd sshd u 41841 + 2.16% swapper kernel k mwait_idle + 1.65% find libc-2.12.so u __GI_vfprintf Something else that is missing in the tui is a header with the names of the columns, that is present in the stdio based formatter. - Arnaldo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL 0/1] perf/core ui browser improvement 2010-09-13 21:29 ` Arnaldo Carvalho de Melo @ 2010-09-14 7:54 ` Peter Zijlstra 2010-09-14 12:16 ` Ingo Molnar 0 siblings, 1 reply; 9+ messages in thread From: Peter Zijlstra @ 2010-09-14 7:54 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ingo Molnar, linux-kernel, Christoph Hellwig, Frederic Weisbecker, Mike Galbraith, Paul Mackerras, Stephane Eranian, Tom Zanussi On Mon, 2010-09-13 at 18:29 -0300, Arnaldo Carvalho de Melo wrote: > + 3.00% find [kernel.kallsyms] [k] n_tty_write > + 3.00% find kernel k n_tty_write I find that [k] is visually easier to separate from the symbol name. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL 0/1] perf/core ui browser improvement 2010-09-14 7:54 ` Peter Zijlstra @ 2010-09-14 12:16 ` Ingo Molnar 2010-09-14 13:07 ` Frederic Weisbecker 0 siblings, 1 reply; 9+ messages in thread From: Ingo Molnar @ 2010-09-14 12:16 UTC (permalink / raw) To: Peter Zijlstra Cc: Arnaldo Carvalho de Melo, linux-kernel, Christoph Hellwig, Frederic Weisbecker, Mike Galbraith, Paul Mackerras, Stephane Eranian, Tom Zanussi * Peter Zijlstra <peterz@infradead.org> wrote: > On Mon, 2010-09-13 at 18:29 -0300, Arnaldo Carvalho de Melo wrote: > > + 3.00% find [kernel.kallsyms] [k] n_tty_write > > > + 3.00% find kernel k n_tty_write > > I find that [k] is visually easier to separate from the symbol name. me too. Something like: before: 1.08% cc1 libc-2.12.so [.] _int_malloc 0.94% cc1 [kernel.kallsyms] [k] clear_page_c 0.74% cc1 libc-2.12.so [.] _int_free 0.69% cc1 [kernel.kallsyms] [k] page_fault 0.46% cc1 libc-2.12.so [.] malloc_consolidate after: 1.08% cc1 # libc-2.12.so [.] _int_malloc 0.94% cc1 # kernel [k] clear_page_c 0.74% cc1 # libc-2.12.so [.] _int_free 0.69% cc1 # kernel [k] page_fault 0.46% cc1 # libc-2.12.so [.] malloc_consolidate Would largely do the trick i think. The comments delineate the DSO portion from the task portion nicely - while also being a familar pattern. The column of [.] and [k] gives kernel developers a quick glance wrt. whether it's kernel or user-space - and it also delineates the symbol column from the DSO column. Thanks, Ingo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL 0/1] perf/core ui browser improvement 2010-09-14 12:16 ` Ingo Molnar @ 2010-09-14 13:07 ` Frederic Weisbecker 0 siblings, 0 replies; 9+ messages in thread From: Frederic Weisbecker @ 2010-09-14 13:07 UTC (permalink / raw) To: Ingo Molnar Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, linux-kernel, Christoph Hellwig, Mike Galbraith, Paul Mackerras, Stephane Eranian, Tom Zanussi On Tue, Sep 14, 2010 at 02:16:11PM +0200, Ingo Molnar wrote: > > * Peter Zijlstra <peterz@infradead.org> wrote: > > > On Mon, 2010-09-13 at 18:29 -0300, Arnaldo Carvalho de Melo wrote: > > > + 3.00% find [kernel.kallsyms] [k] n_tty_write > > > > > + 3.00% find kernel k n_tty_write > > > > I find that [k] is visually easier to separate from the symbol name. > > me too. Something like: > > before: > > 1.08% cc1 libc-2.12.so [.] _int_malloc > 0.94% cc1 [kernel.kallsyms] [k] clear_page_c > 0.74% cc1 libc-2.12.so [.] _int_free > 0.69% cc1 [kernel.kallsyms] [k] page_fault > 0.46% cc1 libc-2.12.so [.] malloc_consolidate > > after: > > 1.08% cc1 # libc-2.12.so [.] _int_malloc > 0.94% cc1 # kernel [k] clear_page_c > 0.74% cc1 # libc-2.12.so [.] _int_free > 0.69% cc1 # kernel [k] page_fault > 0.46% cc1 # libc-2.12.so [.] malloc_consolidate > > > Would largely do the trick i think. The comments delineate the DSO > portion from the task portion nicely - while also being a familar > pattern. The column of [.] and [k] gives kernel developers a quick > glance wrt. whether it's kernel or user-space - and it also delineates > the symbol column from the DSO column. > > Thanks, > > Ingo <random ideas for the future> BTW, having [u] instead of [.] would perhaps be more clearer? And one day we might want to extend that with more granularity, like major contexts in uppercase: K=kernel, U=user, H=hypervisor and some minor in lowercase: h=hardirq, s=softirq, t=task. So you can get [Kh], [Ks], etc... User could then sort by context, which could be useful without callchains for example. </random ideas for the future> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-09-14 13:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-09-13 16:34 [GIT PULL 0/1] perf/core ui browser improvement Arnaldo Carvalho de Melo 2010-09-13 16:34 ` [PATCH 1/1] perf ui browser: Don't use windows, slang is enough Arnaldo Carvalho de Melo 2010-09-13 17:48 ` [GIT PULL 0/1] perf/core ui browser improvement Ingo Molnar 2010-09-13 19:07 ` Arnaldo Carvalho de Melo 2010-09-13 20:42 ` Ingo Molnar 2010-09-13 21:29 ` Arnaldo Carvalho de Melo 2010-09-14 7:54 ` Peter Zijlstra 2010-09-14 12:16 ` Ingo Molnar 2010-09-14 13:07 ` Frederic Weisbecker
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.