Re: [PATCH 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui
On Mon, Apr 20, 2015 at 04:44:11PM -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Apr 20, 2015 at 10:00:46PM +0900, Namhyung Kim escreveu: > > +++ b/tools/perf/ui/browsers/hists.c > > @@ -61,7 +61,7 @@ static int hist_browser__get_folding(struct hist_browser > > *browser) > > rb_entry(nd, struct hist_entry, rb_node); > > > if (he->ms.unfolded) > > - unfolded_rows += he->nr_rows; > > + unfolded_rows += he->tui.nr_rows; > > To avoid all these changes, I wonder if we can't use unamed structs in > addition to the unnamed union? But diff code uses named struct. Does it support a mixed union including named and unnamed members? Hmm.. looks like it is. #include #include struct hist_entry { union { struct /* tui */ { int nr_rows; int row_offset; /* other fields */ }; struct /* diff */ { bool computed; /* other fields */ } diff; }; }; int main(int argc, char *argv[]) { struct hist_entry he = { .nr_rows = 11, .row_offset = 19, }; printf("he.nr_rows=%d, he.row_offset=%d, he.diff.computed=%d\n", he.nr_rows, he.row_offset, he.diff.computed); } namhyung@sejong:tmp$ make mixed cc mixed.c -o mixed namhyung@sejong:tmp$ ./mixed he.nr_rows=11, he.row_offset=19, he.diff.computed=11 OK, I'll change it to unnamed struct. Thanks, Namhyung > > Like this what is done in include/net/sock.h, struct sock_common, and > here: > > [root@zoo ~]# cat unnamed_struct_union.c > #include > #include > > struct hist_entry { > union { > struct /* tui */ { > int nr_rows; > int row_offset; > /* other fields */ > }; > struct /* diff */ { > bool computed; > /* other fields */ > }; > }; > }; > > int main(int argc, char *argv[]) > { > struct hist_entry he = { .nr_rows = 11, .row_offset = 19, }; > > printf("he.nr_rows=%d, he.row_offset=%d\n", he.nr_rows, > he.row_offset); > } > [root@zoo ~]# make unnamed_struct_union > cc unnamed_struct_union.c -o unnamed_struct_union > [root@zoo ~]# ./unnamed_struct_union > he.nr_rows=11, he.row_offset=19 > [root@zoo ~]# > > - 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/ -- 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 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui
On Mon, Apr 20, 2015 at 04:44:11PM -0300, Arnaldo Carvalho de Melo wrote: Em Mon, Apr 20, 2015 at 10:00:46PM +0900, Namhyung Kim escreveu: +++ b/tools/perf/ui/browsers/hists.c @@ -61,7 +61,7 @@ static int hist_browser__get_folding(struct hist_browser *browser) rb_entry(nd, struct hist_entry, rb_node); if (he-ms.unfolded) - unfolded_rows += he-nr_rows; + unfolded_rows += he-tui.nr_rows; To avoid all these changes, I wonder if we can't use unamed structs in addition to the unnamed union? But diff code uses named struct. Does it support a mixed union including named and unnamed members? Hmm.. looks like it is. #include stdbool.h #include stdio.h struct hist_entry { union { struct /* tui */ { int nr_rows; int row_offset; /* other fields */ }; struct /* diff */ { bool computed; /* other fields */ } diff; }; }; int main(int argc, char *argv[]) { struct hist_entry he = { .nr_rows = 11, .row_offset = 19, }; printf(he.nr_rows=%d, he.row_offset=%d, he.diff.computed=%d\n, he.nr_rows, he.row_offset, he.diff.computed); } namhyung@sejong:tmp$ make mixed cc mixed.c -o mixed namhyung@sejong:tmp$ ./mixed he.nr_rows=11, he.row_offset=19, he.diff.computed=11 OK, I'll change it to unnamed struct. Thanks, Namhyung Like this what is done in include/net/sock.h, struct sock_common, and here: [root@zoo ~]# cat unnamed_struct_union.c #include stdbool.h #include stdio.h struct hist_entry { union { struct /* tui */ { int nr_rows; int row_offset; /* other fields */ }; struct /* diff */ { bool computed; /* other fields */ }; }; }; int main(int argc, char *argv[]) { struct hist_entry he = { .nr_rows = 11, .row_offset = 19, }; printf(he.nr_rows=%d, he.row_offset=%d\n, he.nr_rows, he.row_offset); } [root@zoo ~]# make unnamed_struct_union cc unnamed_struct_union.c -o unnamed_struct_union [root@zoo ~]# ./unnamed_struct_union he.nr_rows=11, he.row_offset=19 [root@zoo ~]# - 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/ -- 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 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui
Em Mon, Apr 20, 2015 at 10:00:46PM +0900, Namhyung Kim escreveu: > +++ b/tools/perf/ui/browsers/hists.c > @@ -61,7 +61,7 @@ static int hist_browser__get_folding(struct hist_browser > *browser) > rb_entry(nd, struct hist_entry, rb_node); > if (he->ms.unfolded) > - unfolded_rows += he->nr_rows; > + unfolded_rows += he->tui.nr_rows; To avoid all these changes, I wonder if we can't use unamed structs in addition to the unnamed union? Like this what is done in include/net/sock.h, struct sock_common, and here: [root@zoo ~]# cat unnamed_struct_union.c #include #include struct hist_entry { union { struct /* tui */ { int nr_rows; int row_offset; /* other fields */ }; struct /* diff */ { bool computed; /* other fields */ }; }; }; int main(int argc, char *argv[]) { struct hist_entry he = { .nr_rows = 11, .row_offset = 19, }; printf("he.nr_rows=%d, he.row_offset=%d\n", he.nr_rows, he.row_offset); } [root@zoo ~]# make unnamed_struct_union cc unnamed_struct_union.c -o unnamed_struct_union [root@zoo ~]# ./unnamed_struct_union he.nr_rows=11, he.row_offset=19 [root@zoo ~]# - 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 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui
Hi Jiri, On Mon, Apr 20, 2015 at 10:20:02AM +0200, Jiri Olsa wrote: > On Sun, Apr 19, 2015 at 01:04:11PM +0900, Namhyung Kim wrote: > > Since perf diff only support stdio output, TUI fields are only accessed > > from perf report (or perf top). So add new struct hist_entry_tui and > > move those fields into them. And include it as an union member. > > > > SNIP > > > > > - /* XXX These two should move to some tree widget lib */ > > - u16 row_offset; > > - u16 nr_rows; > > - > > boolinit_have_children; > > charlevel; > > u8 filtered; > > + union { > > + struct hist_entry_diff diff; > > + struct hist_entry_tui tui; > > + }; > > could you please add the comment from changelog in here, > so it's easy to figure this out in the future OK, will do. Thanks, Namhyung -- 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 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui
On Sun, Apr 19, 2015 at 01:04:11PM +0900, Namhyung Kim wrote: > Since perf diff only support stdio output, TUI fields are only accessed > from perf report (or perf top). So add new struct hist_entry_tui and > move those fields into them. And include it as an union member. > SNIP > > - /* XXX These two should move to some tree widget lib */ > - u16 row_offset; > - u16 nr_rows; > - > boolinit_have_children; > charlevel; > u8 filtered; > + union { > + struct hist_entry_diff diff; > + struct hist_entry_tui tui; > + }; could you please add the comment from changelog in here, so it's easy to figure this out in the future thanks, jirka -- 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 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui
Hi Jiri, On Mon, Apr 20, 2015 at 10:20:02AM +0200, Jiri Olsa wrote: On Sun, Apr 19, 2015 at 01:04:11PM +0900, Namhyung Kim wrote: Since perf diff only support stdio output, TUI fields are only accessed from perf report (or perf top). So add new struct hist_entry_tui and move those fields into them. And include it as an union member. SNIP - /* XXX These two should move to some tree widget lib */ - u16 row_offset; - u16 nr_rows; - boolinit_have_children; charlevel; u8 filtered; + union { + struct hist_entry_diff diff; + struct hist_entry_tui tui; + }; could you please add the comment from changelog in here, so it's easy to figure this out in the future OK, will do. Thanks, Namhyung -- 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 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui
On Sun, Apr 19, 2015 at 01:04:11PM +0900, Namhyung Kim wrote: Since perf diff only support stdio output, TUI fields are only accessed from perf report (or perf top). So add new struct hist_entry_tui and move those fields into them. And include it as an union member. SNIP - /* XXX These two should move to some tree widget lib */ - u16 row_offset; - u16 nr_rows; - boolinit_have_children; charlevel; u8 filtered; + union { + struct hist_entry_diff diff; + struct hist_entry_tui tui; + }; could you please add the comment from changelog in here, so it's easy to figure this out in the future thanks, jirka -- 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 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui
Em Mon, Apr 20, 2015 at 10:00:46PM +0900, Namhyung Kim escreveu: +++ b/tools/perf/ui/browsers/hists.c @@ -61,7 +61,7 @@ static int hist_browser__get_folding(struct hist_browser *browser) rb_entry(nd, struct hist_entry, rb_node); if (he-ms.unfolded) - unfolded_rows += he-nr_rows; + unfolded_rows += he-tui.nr_rows; To avoid all these changes, I wonder if we can't use unamed structs in addition to the unnamed union? Like this what is done in include/net/sock.h, struct sock_common, and here: [root@zoo ~]# cat unnamed_struct_union.c #include stdbool.h #include stdio.h struct hist_entry { union { struct /* tui */ { int nr_rows; int row_offset; /* other fields */ }; struct /* diff */ { bool computed; /* other fields */ }; }; }; int main(int argc, char *argv[]) { struct hist_entry he = { .nr_rows = 11, .row_offset = 19, }; printf(he.nr_rows=%d, he.row_offset=%d\n, he.nr_rows, he.row_offset); } [root@zoo ~]# make unnamed_struct_union cc unnamed_struct_union.c -o unnamed_struct_union [root@zoo ~]# ./unnamed_struct_union he.nr_rows=11, he.row_offset=19 [root@zoo ~]# - 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/