search

ARM-software / trappy

This repository has moved to https://gitlab.arm.com/tooling/trappy
Apache License 2.0
60 stars 39 forks source link

[RFC] Systrace's custom events formatting #243

Open derkling opened 7 years ago

derkling commented 7 years ago

The Android run-time can inject interesting function profiling events from user-space which unfortunately are not formatted using the key=value pairs required by TRAPpy.

The format string it uses is expected by other tools, e.g. systrace, thus changing the formatting at the source can be much more complex than dealing with in in TRAPpy. Since TRAPpy learnt to parse systrace events, we can try to improve its skills by properly parsing these custom events too.

The basic idea is to add a simple call-back based mechanism which allows a specific parser (i.e. SysTrace ) to reformat a data string into a TRAPpy compliant format before processing it the usual way.

This is an initial prototype which I share to get feedback on the overall approach before going further to develop more complex functionalities. Thus, I've not yet added a test for this feature, however a simple example trace can contain these events:

RenderThread-9568  ( 9546) [006] ...1  3210.844141: tracing_mark_write: B|9546|flush drawing commands
hwuiTask1-9571 ( 9546) [007] ...1  3210.844145: tracing_mark_write: E
hwuiTask1-9571 ( 9546) [007] ...1  3210.844151: tracing_mark_write: B|9546|tessellateAmbientShadow
hwuiTask1-9571 ( 9546) [007] ...1  3210.844159: tracing_mark_write: E
hwuiTask1-9571 ( 9546) [007] ...1  3210.844162: tracing_mark_write: B|9546|tessellateSpotShadow
hwuiTask1-9571 ( 9546) [007] ...1  3210.844182: tracing_mark_write: E

As additional goals, not yet provided by this prototype, it would be nice to:

  1. support the generation and registration on-the-fly of new dataframes from within the callback, which should allow to create separate dataframe for different classes of custom events
  2. make sure the systrace viewer and the TRAPpy generated dataframes are on the same timeline, which will require to play with basetime and windows... but, since this is a more complex task, I would keep this topic for a different PR.
derkling commented 7 years ago

The approach @bjackman suggested was already briefly discussed with @JaviMerino and @credp, and it actually I think it can make much sense... I just not implemented it to push out what I had so far and trigger the discussion. Especially in view of @joelagnel PR #241 ... we need to avoid overlapping features and I was wondering if what I'm proposing here should be better implemented using client callbacks.

What do you think? Do you think it can still be valuable for the SysTrace parser to support directly this additional parsing logic?

bjackman commented 7 years ago

Still investigating @joelagnel's PR but I think SysTrace should parse it directly.

joelagnel commented 7 years ago

Sorry for the late review on this. I agree with the discussed approach of using generate_data_dict from the tracer in question. So probably the tracer should pass this function as a callback to the 'class Base' event objects?

@derkling also yes I think there is overlap with #241 but for slightly different purpose, we should definitely use that mechanism, and I think its also good because it might make support for #241 easier. At the moment I am dedicated to solving this systrace parsing problem because we need it for automated trace analysis so lets work together on this and get it done.

bjackman commented 7 years ago

I agree with the discussed approach of using generate_data_dict from the tracer in question. So probably the tracer should pass this function as a callback to the 'class Base' event objects?

Why a callback? Can't it just be an override?

kdub commented 6 years ago

Some examples of a custom parser I wrote recently (https://github.com/ARM-software/trappy/pull/271), if it provides any inspiration.