Re: perf script: Question: Python trace processing script contains the tid of the process in the common_pid attribute

[Arun Kalyanasundaram]

Arnaldo:
So, I think what you are suggesting is, we should check the signature
of the hook to determine if it has an additional attribute and only
then should we provide the dict. May be something like this:

PyObject* custom_dict = PyObject_GetAttrString(handler, "other_fields_dict");
if (custom_dict)
    //Add dict to PyTuple

Do you think this would be a better approach?

Thank you,
- Arun

On Wed, Jul 5, 2017 at 1:41 PM, Arun Kalyanasundaram
<arunkaly@xxxxxxxxxx> wrote:
> I wasn't entirely sure if we should modify the signature of the python
> hooks_  as this would make existing scripts incompatible. So the patch
> only adds sample->pid to the event_fields_dict param in
> trace_unhandled().
>
>
> On Wed, Jul 5, 2017 at 12:26 PM, Arnaldo Carvalho de Melo
> <acme@xxxxxxxxxx> wrote:
>> Em Wed, Jul 05, 2017 at 04:25:45PM -0300, Arnaldo Carvalho de Melo escreveu:
>>> Em Wed, Jul 05, 2017 at 09:22:07AM -0700, Arun Kalyanasundaram escreveu:
>>> > Hi Arnaldo,
>>> >
>>> > Thank you for your reply.
>>> > I actually meant tracepoint event handlers: def
>>> > trace_unhandled(event_name, context, event_fields_dict)
>>> > The dict parameter contains an attribute "common_pid" which is
>>> > actually the "tid" of the thread. There are no other attributes that
>>> > contain the actual pid of the process. So, I was wondering if this is
>>> > something intentional? If not I can share a patch to fix this.
>>>
>>> Yeah there is a problem in:
>>>
>>> tools/perf/util/scripting-engines/trace-event-python.c
>>>
>>> static void python_process_event(union perf_event *event,
>>>                                  struct perf_sample *sample,
>>>                                  struct perf_evsel *evsel,
>>>                                  struct addr_location *al)
>>> {
>>>         struct tables *tables = &tables_global;
>>>
>>>         switch (evsel->attr.type) {
>>>         case PERF_TYPE_TRACEPOINT:
>>>                 python_process_tracepoint(sample, evsel, al);
>>>                 break;
>>>         /* Reserve for future process_hw/sw/raw APIs */
>>>         default:
>>>                 if (tables->db_export_mode)
>>>                         db_export__sample(&tables->dbe, event, sample, evsel, al);
>>>                 else
>>>                         python_process_general_event(sample, evsel, al);
>>>         }
>>> }
>>>
>>> The python_process_tracepoint() thing predates
>>> python_process_general_event(), and doesn't adds the dict with all the
>>> perf_sample entries that python_process_general_event() passes to the
>>> python method :-\
>>>
>>> Both the per-tracepoint python hooks _and_ trace_unhandled() should get
>>> that dict, is that what your patch does?
>>
>> Well, for performance reasons I think perhaps we could take a look at
>> the signature of the python hook and provide the dictionary only if it
>> is in it?
>>
>> - Arnaldo