Re: [PATCH 04/30] perf tools: Add comm string into struct thread_map

Jiri Olsa
2015-06-15T21:56:05Z

On Mon, Jun 15, 2015 at 04:17:07PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Jun 14, 2015 at 10:19:19AM +0200, Jiri Olsa escreveu:
> > Adding support to hold comm name together with pids in
> > 'struct thread_map'. It will be useful for --per-task
> > option to display task pid together with task name.
> >
> > Getting the task name from /proc/$pid/comm.
>
> Humm, so you want to extend the thread_map, that was just for pids, to
> have some more info about the thread, if that is the case, why not
> synthesize the whole thread, and have a pointer to 'struct thread', that
> is the representation for a thread in perf instead of creating a new,
> partial thing like 'thread_map_data'?
>
> You wouldn't have to use the existing synthesizing code if that seems to
> overkill, i.e. instead of having 'pid_t' as the array element in a
> thread_map, you would have 'struct thread', i.e. the whole shebang, but
> would just set what you need now, i.e. thread->pid and thread->comm, but
> later could do the rest instead of recreating it in that thread_map_data
> :-)

hum, I guess I could find some parts of code that could be shared,
but I fear not all of it.. 'struct thread' seems to be fairly
customized for usage in sampling report code

maybe introduce 'machine' storage and provide interface to fill in
live server data.. but it feels like overkill just for 2 lines of
code that need print pid's name ;-)

>
> I.e. besides --per-task, don't you want --per-pid, --per-thread, or
> other stuff that will use other thread characteristics?

with --per-task we dont do any grouping, just displaying
what we store (now) for each separated thread

it might be better to use --per-thread instead of --per-task probably,
I dont have strong feeling about that

also we could add --per-process that would group all threads
based on process

jirka