Messages in this thread | | | Date | Thu, 25 Jun 2015 18:01:25 +0200 | From | Jiri Olsa <> | Subject | Re: [PATCH 02/27] perf tools: Add comm string into struct thread_map |
| |
On Thu, Jun 25, 2015 at 11:39:27AM -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, Jun 23, 2015 at 11:56:18PM +0200, Jiri Olsa escreveu: > > On Tue, Jun 23, 2015 at 06:01:58PM -0300, Arnaldo Carvalho de Melo wrote: > > > Em Tue, Jun 23, 2015 at 12:36:03AM +0200, Jiri Olsa escreveu: > > > > Adding support to hold comm name together with pids in > > > > 'struct thread_map'. It will be useful for --per-thread > > > > option to display task pid together with task name. > > > > > Getting the task name from /proc/$pid/comm. > > > > Do we really need to read this all the time? Or just for tools that need > > > this? > > > > Can't we have a thread_map__read_comms(map); > > > > That we can make this specific --per-thread option in 'stat' call? > > > ook > > Thanks, this made me think: thread_map is just an array that enumerates > the pids ultimately for use with sys_perf_event_open(), bloating it with > extra details used by just one oddball subfeature of just one tool seems > overkill. > > Probably it would be better to rename it to pid_map, that is what is > useful for all tools using it to create the event file descriptors and > have a comm_map that would return another enumeration to obtain the > comms, something like: > > struct thread_map *tids = thread__map_by_foo(parameters); > struct comm_map *comms = comm_map__new(tids); > > That way the various ways of populating the thread_map->map[] (the > _by_foo() variants of thread_map we have) would do that leg work and > the comm_map__new(tids) would just read the comms. > > The thread_map users would not have to be touched at all, it would be > kept as simple as it was and just tools that need those comms would go > to the trouble of allocating the needed space for the array and to do > the comm reading. > > But nah, I'll probably merge what you did and later, if I find the time, > do it myself, better to have that new feature than to be picky, right?
right ;-) also the 'comm' string is small update after long time of no change in thread_map and I guess it will take probably another long time period before we will want to change it again..
thanks, jirka
| |