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] trappy: Add tgid information from systrace #248

Closed joelagnel closed 7 years ago

joelagnel commented 7 years ago

Add tgid information if available during parsing traces

Change-Id: I7d280a9dc95d48a2730983e5ddb474d266ebf0fd Signed-off-by: Joel Fernandes joelaf@google.com

joelagnel commented 7 years ago

Does anyone know why the stupid CI fails? The CI logs don't make sense to me. I ran nosetests and it runs fine locally for me.

bjackman commented 7 years ago

I can't repro the issue either - I noticed that I'm running Python 2.7.6 but the CI is on 2.7.3 - maybe there was a bug in re? I didn't find anything relevant in the Python changelog but this StackOverflow question suggests there was indeed a bug.

If I'm right, my initial thought was to just update the CI to use a recent Python (I think we could just change the Travis setup to use a more recent Ubuntu) but actually I suppose we should work around this so people can use Trappy on old systems...

bjackman commented 7 years ago

I haven't read this yet but I guess this is based on your upstream kernel posting? If so do we need to wait for that to get merged?

joelagnel commented 7 years ago

Hi Brendan, We don't have to wait because an older implementation of that has been in Android kernels since years.

On Jun 5, 2017 4:47 AM, "Brendan Jackman" notifications@github.com wrote:

I haven't read this yet but I guess this is based on your upstream kernel posting? If so do we need to wait for that to get merged?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ARM-software/trappy/pull/248#issuecomment-306169001, or mute the thread https://github.com/notifications/unsubscribe-auth/AACSVGNqFNQaJNOYImDuO3Rjeu7XRJT9ks5sA-rhgaJpZM4Nu7Ij .

bjackman commented 7 years ago

Oh OK!

bjackman commented 7 years ago

Aside from the Python regex bug, there are some places in the tests where we assert on the columns of the DataFrames, so presumably they'll need fixing. Also maybe there's some documentation somewhere that lists the special field columns? I can't see it anywhere though.

joelagnel commented 7 years ago

I closed this for now since I've to rewrite it to be completely different and don't want to confuse reviewers.