Re: [PATCH 3/7] perf tools: Move TUI-specific fields to struct hist_entry_tui

2015-04-21 Thread Namhyung Kim
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

2015-04-21 Thread Namhyung Kim
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

2015-04-20 Thread Arnaldo Carvalho de Melo
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

2015-04-20 Thread Namhyung Kim
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

2015-04-20 Thread Jiri Olsa
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

2015-04-20 Thread Namhyung Kim
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

2015-04-20 Thread Jiri Olsa
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

2015-04-20 Thread Arnaldo Carvalho de Melo
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/