Re: perf: allow command to attach local data to thread/evsel structs

[Arnaldo Carvalho de Melo <acme@ghostprotocols.net>]

Em Tue, Feb 07, 2012 at 11:11:48AM -0700, David Ahern escreveu:
> This is an API I have been using for some 'local' commands that process
> perf events. It allows the commands to attach data to events and threads
> and avoid local caching and lookups.

In the kernel proper we try to get away with this pattern using
container_of where possible.

Here tho the structures are created in library functions.

The symbol library has this symbol_conf.priv_size + symbol__priv() to do
what you want while avoiding two allocs + one pointer in a core
structure.

I think we either use {thread_conf,evsel_conf} for global configuration
options for these two core data structures or we just provide some
optional, per perf_tool allocator.

Yeah, that sounds extreme, but hey, this is a profiler, we ought to eat
our own dog food, right?

- Arnaldo

P.S.: Are your tools too specific or are they upstreamable?
 
> 
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 326b8e4..866de40 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -66,6 +66,12 @@ struct perf_evsel {
>         void        *data;
>     } handler;
>     bool            supported;
> +
> +   /*
> +    * can be used by commands that process samples
> +    * for storing local, event-based data
> +    */
> +   void *private;
>  };
> 
>  struct cpu_map;
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 70c2c13..9f62859 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -16,6 +16,12 @@ struct thread {
>     bool            comm_set;
>     char            *comm;
>     int         comm_len;
> +
> +    /*
> +    * can be used by commands that process samples
> +    * for storing local, thread-based data
> +    */
> +   void *private;
>  };
> 
>  struct machine;
> 
> One wrinkle is that for the thread-based one the thread__delete(),
> thread__fork() and thread__set_comm() functions would free the
> allocations if set -- or a handler is needed to free private structs to
> handle layered mallocs.
> 
> Any objections to pushing this kind of open-ended API into perf?
> 
> David