Re: [PATCH v2 2/2] perf hists browser: Consolidate callchain print functions in TUI
Em Fri, Aug 22, 2014 at 10:02:36AM +0200, Ingo Molnar escreveu:
> > > +struct callchain_print_arg {
> > > + /* for hists browser */
> > > + unsigned short row;
> > > + off_t row_offset;
> > > + bool is_current_entry;
> > > +
> > > + /* for file dump */
> > > + FILE *fp;
> > > + int printed;
> > > +};
>
> Just a data type definition nitpicking pet peeve of mine, don't
> you guys too find this vertically aligned form infinitely more
> readable:
>
> struct callchain_print_arg {
> /* for hists browser */
> unsigned short row;
> off_t row_offset;
> boolis_current_entry;
>
> /* for file dump */
> FILE*fp;
> int printed;
> };
>
> especially when looking at it via email, without syntax
> highlighting?
Agreed, will make it so when applying it.
- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] perf hists browser: Consolidate callchain print functions in TUI
* Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 21, 2014 at 10:15:46AM +0900, Namhyung Kim escreveu:
> > Currently there're two callchain print functions in TUI - one for the
> > hists browser and another for file dump. They do almost same job so
> > it'd be better consolidate the codes.
>
> Some comments below, thanks for doing this work!
>
> > To do that, move row calculation code into a print callback so that
> > the dump code cannot be limited by the current screen size.
>
> > Cc: Frederic Weisbecker
> > Signed-off-by: Namhyung Kim
> > ---
> > tools/perf/ui/browsers/hists.c | 210
> > +++--
> > 1 file changed, 76 insertions(+), 134 deletions(-)
> >
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index 519353d9f5fb..48d8c8eee6c2 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -477,20 +477,32 @@ static char *callchain_list__sym_name(struct
> > callchain_list *cl,
> > return bf;
> > }
> >
> > +struct callchain_print_arg {
> > + /* for hists browser */
> > + unsigned short row;
> > + off_t row_offset;
> > + bool is_current_entry;
> > +
> > + /* for file dump */
> > + FILE *fp;
> > + int printed;
> > +};
Just a data type definition nitpicking pet peeve of mine, don't
you guys too find this vertically aligned form infinitely more
readable:
struct callchain_print_arg {
/* for hists browser */
unsigned short row;
off_t row_offset;
boolis_current_entry;
/* for file dump */
FILE*fp;
int printed;
};
especially when looking at it via email, without syntax
highlighting?
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] perf hists browser: Consolidate callchain print functions in TUI
* Arnaldo Carvalho de Melo a...@kernel.org wrote:
Em Thu, Aug 21, 2014 at 10:15:46AM +0900, Namhyung Kim escreveu:
Currently there're two callchain print functions in TUI - one for the
hists browser and another for file dump. They do almost same job so
it'd be better consolidate the codes.
Some comments below, thanks for doing this work!
To do that, move row calculation code into a print callback so that
the dump code cannot be limited by the current screen size.
Cc: Frederic Weisbecker fweis...@gmail.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
tools/perf/ui/browsers/hists.c | 210
+++--
1 file changed, 76 insertions(+), 134 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 519353d9f5fb..48d8c8eee6c2 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -477,20 +477,32 @@ static char *callchain_list__sym_name(struct
callchain_list *cl,
return bf;
}
+struct callchain_print_arg {
+ /* for hists browser */
+ unsigned short row;
+ off_t row_offset;
+ bool is_current_entry;
+
+ /* for file dump */
+ FILE *fp;
+ int printed;
+};
Just a data type definition nitpicking pet peeve of mine, don't
you guys too find this vertically aligned form infinitely more
readable:
struct callchain_print_arg {
/* for hists browser */
unsigned short row;
off_t row_offset;
boolis_current_entry;
/* for file dump */
FILE*fp;
int printed;
};
especially when looking at it via email, without syntax
highlighting?
Thanks,
Ingo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] perf hists browser: Consolidate callchain print functions in TUI
Em Fri, Aug 22, 2014 at 10:02:36AM +0200, Ingo Molnar escreveu:
+struct callchain_print_arg {
+ /* for hists browser */
+ unsigned short row;
+ off_t row_offset;
+ bool is_current_entry;
+
+ /* for file dump */
+ FILE *fp;
+ int printed;
+};
Just a data type definition nitpicking pet peeve of mine, don't
you guys too find this vertically aligned form infinitely more
readable:
struct callchain_print_arg {
/* for hists browser */
unsigned short row;
off_t row_offset;
boolis_current_entry;
/* for file dump */
FILE*fp;
int printed;
};
especially when looking at it via email, without syntax
highlighting?
Agreed, will make it so when applying it.
- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] perf hists browser: Consolidate callchain print functions in TUI
Em Thu, Aug 21, 2014 at 10:15:46AM +0900, Namhyung Kim escreveu:
> Currently there're two callchain print functions in TUI - one for the
> hists browser and another for file dump. They do almost same job so
> it'd be better consolidate the codes.
Some comments below, thanks for doing this work!
> To do that, move row calculation code into a print callback so that
> the dump code cannot be limited by the current screen size.
> Cc: Frederic Weisbecker
> Signed-off-by: Namhyung Kim
> ---
> tools/perf/ui/browsers/hists.c | 210
> +++--
> 1 file changed, 76 insertions(+), 134 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index 519353d9f5fb..48d8c8eee6c2 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -477,20 +477,32 @@ static char *callchain_list__sym_name(struct
> callchain_list *cl,
> return bf;
> }
>
> +struct callchain_print_arg {
> + /* for hists browser */
> + unsigned short row;
> + off_t row_offset;
> + bool is_current_entry;
> +
> + /* for file dump */
> + FILE *fp;
> + int printed;
> +};
> +
> static void hist_browser__show_callchain_entry(struct hist_browser *browser,
> struct callchain_list *chain,
> -unsigned short row, int offset,
> -char folded_sign, const char
> *str,
> -bool *is_current_entry)
> +const char *str, int offset,
> +struct callchain_print_arg *arg)
> {
> int color, width;
> + unsigned short row = arg->row;
> + char folded_sign = callchain_list__folded(chain);
>
> color = HE_COLORSET_NORMAL;
> width = browser->b.width - (offset + 2);
> if (ui_browser__is_current_entry(>b, row)) {
> browser->selection = >ms;
> color = HE_COLORSET_SELECTED;
> - *is_current_entry = true;
> + arg->is_current_entry = true;
> }
>
> ui_browser__set_color(>b, color);
> @@ -498,17 +510,40 @@ static void hist_browser__show_callchain_entry(struct
> hist_browser *browser,
> slsmg_write_nstring(" ", offset);
> slsmg_printf("%c ", folded_sign);
> slsmg_write_nstring(str, width);
> +
> + /*
> + * increase row here so that we can reuse the
> + * hist_browser__show_callchain() for dumping the whole
> + * callchain to a file.
> + */
> + arg->row++;
> +}
> +
> +static void hist_browser__fprintf_callchain_entry(struct hist_browser *b
> __maybe_unused,
> + struct callchain_list *chain,
> + const char *str, int offset,
> + struct callchain_print_arg
> *arg)
> +{
> + char folded_sign = callchain_list__folded(chain);
> +
> + arg->printed += fprintf(arg->fp, "%*s%c %s\n", offset, " ",
> + folded_sign, str);
> }
>
> +typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
> + struct callchain_list *chain,
> + const char *str, int offset,
> + struct callchain_print_arg *arg);
> +
> #define LEVEL_OFFSET_STEP 3
>
> static int hist_browser__show_callchain(struct hist_browser *browser,
> - struct rb_root *root, int level,
> - unsigned short row, off_t *row_offset,
> - u64 total, bool *is_current_entry)
> + struct rb_root *root, int level, u64
> total,
> + print_callchain_entry_fn print,
> + struct callchain_print_arg *arg)
> {
> struct rb_node *node;
> - int first_row = row, offset = level * LEVEL_OFFSET_STEP;
> + int first_row = arg->row, offset = level * LEVEL_OFFSET_STEP;
> u64 new_total;
>
> node = rb_first(root);
> @@ -532,8 +567,8 @@ static int hist_browser__show_callchain(struct
> hist_browser *browser,
> extra_offset = LEVEL_OFFSET_STEP;
>
> folded_sign = callchain_list__folded(chain);
> - if (*row_offset != 0) {
> - --*row_offset;
> + if (arg->row_offset != 0) {
> + arg->row_offset--;
> goto do_next;
> }
>
> @@ -550,13 +585,11 @@ static int hist_browser__show_callchain(struct
> hist_browser *browser,
> str = alloc_str;
> }
>
> -
Re: [PATCH v2 2/2] perf hists browser: Consolidate callchain print functions in TUI
Em Thu, Aug 21, 2014 at 10:15:46AM +0900, Namhyung Kim escreveu:
Currently there're two callchain print functions in TUI - one for the
hists browser and another for file dump. They do almost same job so
it'd be better consolidate the codes.
Some comments below, thanks for doing this work!
To do that, move row calculation code into a print callback so that
the dump code cannot be limited by the current screen size.
Cc: Frederic Weisbecker fweis...@gmail.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
tools/perf/ui/browsers/hists.c | 210
+++--
1 file changed, 76 insertions(+), 134 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 519353d9f5fb..48d8c8eee6c2 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -477,20 +477,32 @@ static char *callchain_list__sym_name(struct
callchain_list *cl,
return bf;
}
+struct callchain_print_arg {
+ /* for hists browser */
+ unsigned short row;
+ off_t row_offset;
+ bool is_current_entry;
+
+ /* for file dump */
+ FILE *fp;
+ int printed;
+};
+
static void hist_browser__show_callchain_entry(struct hist_browser *browser,
struct callchain_list *chain,
-unsigned short row, int offset,
-char folded_sign, const char
*str,
-bool *is_current_entry)
+const char *str, int offset,
+struct callchain_print_arg *arg)
{
int color, width;
+ unsigned short row = arg-row;
+ char folded_sign = callchain_list__folded(chain);
color = HE_COLORSET_NORMAL;
width = browser-b.width - (offset + 2);
if (ui_browser__is_current_entry(browser-b, row)) {
browser-selection = chain-ms;
color = HE_COLORSET_SELECTED;
- *is_current_entry = true;
+ arg-is_current_entry = true;
}
ui_browser__set_color(browser-b, color);
@@ -498,17 +510,40 @@ static void hist_browser__show_callchain_entry(struct
hist_browser *browser,
slsmg_write_nstring( , offset);
slsmg_printf(%c , folded_sign);
slsmg_write_nstring(str, width);
+
+ /*
+ * increase row here so that we can reuse the
+ * hist_browser__show_callchain() for dumping the whole
+ * callchain to a file.
+ */
+ arg-row++;
+}
+
+static void hist_browser__fprintf_callchain_entry(struct hist_browser *b
__maybe_unused,
+ struct callchain_list *chain,
+ const char *str, int offset,
+ struct callchain_print_arg
*arg)
+{
+ char folded_sign = callchain_list__folded(chain);
+
+ arg-printed += fprintf(arg-fp, %*s%c %s\n, offset, ,
+ folded_sign, str);
}
+typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
+ struct callchain_list *chain,
+ const char *str, int offset,
+ struct callchain_print_arg *arg);
+
#define LEVEL_OFFSET_STEP 3
static int hist_browser__show_callchain(struct hist_browser *browser,
- struct rb_root *root, int level,
- unsigned short row, off_t *row_offset,
- u64 total, bool *is_current_entry)
+ struct rb_root *root, int level, u64
total,
+ print_callchain_entry_fn print,
+ struct callchain_print_arg *arg)
{
struct rb_node *node;
- int first_row = row, offset = level * LEVEL_OFFSET_STEP;
+ int first_row = arg-row, offset = level * LEVEL_OFFSET_STEP;
u64 new_total;
node = rb_first(root);
@@ -532,8 +567,8 @@ static int hist_browser__show_callchain(struct
hist_browser *browser,
extra_offset = LEVEL_OFFSET_STEP;
folded_sign = callchain_list__folded(chain);
- if (*row_offset != 0) {
- --*row_offset;
+ if (arg-row_offset != 0) {
+ arg-row_offset--;
goto do_next;
}
@@ -550,13 +585,11 @@ static int hist_browser__show_callchain(struct
hist_browser *browser,
str = alloc_str;
}
- hist_browser__show_callchain_entry(browser, chain, row,
-
[PATCH v2 2/2] perf hists browser: Consolidate callchain print functions in TUI
Currently there're two callchain print functions in TUI - one for the
hists browser and another for file dump. They do almost same job so
it'd be better consolidate the codes.
To do that, move row calculation code into a print callback so that
the dump code cannot be limited by the current screen size.
Cc: Frederic Weisbecker
Signed-off-by: Namhyung Kim
---
tools/perf/ui/browsers/hists.c | 210 +++--
1 file changed, 76 insertions(+), 134 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 519353d9f5fb..48d8c8eee6c2 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -477,20 +477,32 @@ static char *callchain_list__sym_name(struct
callchain_list *cl,
return bf;
}
+struct callchain_print_arg {
+ /* for hists browser */
+ unsigned short row;
+ off_t row_offset;
+ bool is_current_entry;
+
+ /* for file dump */
+ FILE *fp;
+ int printed;
+};
+
static void hist_browser__show_callchain_entry(struct hist_browser *browser,
struct callchain_list *chain,
- unsigned short row, int offset,
- char folded_sign, const char
*str,
- bool *is_current_entry)
+ const char *str, int offset,
+ struct callchain_print_arg *arg)
{
int color, width;
+ unsigned short row = arg->row;
+ char folded_sign = callchain_list__folded(chain);
color = HE_COLORSET_NORMAL;
width = browser->b.width - (offset + 2);
if (ui_browser__is_current_entry(>b, row)) {
browser->selection = >ms;
color = HE_COLORSET_SELECTED;
- *is_current_entry = true;
+ arg->is_current_entry = true;
}
ui_browser__set_color(>b, color);
@@ -498,17 +510,40 @@ static void hist_browser__show_callchain_entry(struct
hist_browser *browser,
slsmg_write_nstring(" ", offset);
slsmg_printf("%c ", folded_sign);
slsmg_write_nstring(str, width);
+
+ /*
+* increase row here so that we can reuse the
+* hist_browser__show_callchain() for dumping the whole
+* callchain to a file.
+*/
+ arg->row++;
+}
+
+static void hist_browser__fprintf_callchain_entry(struct hist_browser *b
__maybe_unused,
+ struct callchain_list *chain,
+ const char *str, int offset,
+ struct callchain_print_arg
*arg)
+{
+ char folded_sign = callchain_list__folded(chain);
+
+ arg->printed += fprintf(arg->fp, "%*s%c %s\n", offset, " ",
+ folded_sign, str);
}
+typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
+struct callchain_list *chain,
+const char *str, int offset,
+struct callchain_print_arg *arg);
+
#define LEVEL_OFFSET_STEP 3
static int hist_browser__show_callchain(struct hist_browser *browser,
- struct rb_root *root, int level,
- unsigned short row, off_t *row_offset,
- u64 total, bool *is_current_entry)
+ struct rb_root *root, int level, u64
total,
+ print_callchain_entry_fn print,
+ struct callchain_print_arg *arg)
{
struct rb_node *node;
- int first_row = row, offset = level * LEVEL_OFFSET_STEP;
+ int first_row = arg->row, offset = level * LEVEL_OFFSET_STEP;
u64 new_total;
node = rb_first(root);
@@ -532,8 +567,8 @@ static int hist_browser__show_callchain(struct hist_browser
*browser,
extra_offset = LEVEL_OFFSET_STEP;
folded_sign = callchain_list__folded(chain);
- if (*row_offset != 0) {
- --*row_offset;
+ if (arg->row_offset != 0) {
+ arg->row_offset--;
goto do_next;
}
@@ -550,13 +585,11 @@ static int hist_browser__show_callchain(struct
hist_browser *browser,
str = alloc_str;
}
- hist_browser__show_callchain_entry(browser, chain, row,
- offset +
extra_offset,
- folded_sign,
[PATCH v2 2/2] perf hists browser: Consolidate callchain print functions in TUI
Currently there're two callchain print functions in TUI - one for the
hists browser and another for file dump. They do almost same job so
it'd be better consolidate the codes.
To do that, move row calculation code into a print callback so that
the dump code cannot be limited by the current screen size.
Cc: Frederic Weisbecker fweis...@gmail.com
Signed-off-by: Namhyung Kim namhy...@kernel.org
---
tools/perf/ui/browsers/hists.c | 210 +++--
1 file changed, 76 insertions(+), 134 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 519353d9f5fb..48d8c8eee6c2 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -477,20 +477,32 @@ static char *callchain_list__sym_name(struct
callchain_list *cl,
return bf;
}
+struct callchain_print_arg {
+ /* for hists browser */
+ unsigned short row;
+ off_t row_offset;
+ bool is_current_entry;
+
+ /* for file dump */
+ FILE *fp;
+ int printed;
+};
+
static void hist_browser__show_callchain_entry(struct hist_browser *browser,
struct callchain_list *chain,
- unsigned short row, int offset,
- char folded_sign, const char
*str,
- bool *is_current_entry)
+ const char *str, int offset,
+ struct callchain_print_arg *arg)
{
int color, width;
+ unsigned short row = arg-row;
+ char folded_sign = callchain_list__folded(chain);
color = HE_COLORSET_NORMAL;
width = browser-b.width - (offset + 2);
if (ui_browser__is_current_entry(browser-b, row)) {
browser-selection = chain-ms;
color = HE_COLORSET_SELECTED;
- *is_current_entry = true;
+ arg-is_current_entry = true;
}
ui_browser__set_color(browser-b, color);
@@ -498,17 +510,40 @@ static void hist_browser__show_callchain_entry(struct
hist_browser *browser,
slsmg_write_nstring( , offset);
slsmg_printf(%c , folded_sign);
slsmg_write_nstring(str, width);
+
+ /*
+* increase row here so that we can reuse the
+* hist_browser__show_callchain() for dumping the whole
+* callchain to a file.
+*/
+ arg-row++;
+}
+
+static void hist_browser__fprintf_callchain_entry(struct hist_browser *b
__maybe_unused,
+ struct callchain_list *chain,
+ const char *str, int offset,
+ struct callchain_print_arg
*arg)
+{
+ char folded_sign = callchain_list__folded(chain);
+
+ arg-printed += fprintf(arg-fp, %*s%c %s\n, offset, ,
+ folded_sign, str);
}
+typedef void (*print_callchain_entry_fn)(struct hist_browser *browser,
+struct callchain_list *chain,
+const char *str, int offset,
+struct callchain_print_arg *arg);
+
#define LEVEL_OFFSET_STEP 3
static int hist_browser__show_callchain(struct hist_browser *browser,
- struct rb_root *root, int level,
- unsigned short row, off_t *row_offset,
- u64 total, bool *is_current_entry)
+ struct rb_root *root, int level, u64
total,
+ print_callchain_entry_fn print,
+ struct callchain_print_arg *arg)
{
struct rb_node *node;
- int first_row = row, offset = level * LEVEL_OFFSET_STEP;
+ int first_row = arg-row, offset = level * LEVEL_OFFSET_STEP;
u64 new_total;
node = rb_first(root);
@@ -532,8 +567,8 @@ static int hist_browser__show_callchain(struct hist_browser
*browser,
extra_offset = LEVEL_OFFSET_STEP;
folded_sign = callchain_list__folded(chain);
- if (*row_offset != 0) {
- --*row_offset;
+ if (arg-row_offset != 0) {
+ arg-row_offset--;
goto do_next;
}
@@ -550,13 +585,11 @@ static int hist_browser__show_callchain(struct
hist_browser *browser,
str = alloc_str;
}
- hist_browser__show_callchain_entry(browser, chain, row,
- offset +
extra_offset,
-
