Handle sched_wakeup in Linux perf importer

Handle sched_wakeup in Linux perf importer

BUG=13

Committed: https://code.google.com/p/trace-viewer/source/detail?r=210

[jmg, 2013-02-15 03:29:06]

I found the change I originally did for bug 13.

[dsinclair, 2013-02-15 04:10:42]

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js
File src/linux_perf_importer.js (right):

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:364: if (!thread.tempWakeups)
thread.tempWakeups = thread.tempWakeups || [];

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:388: }
Can this be shortened too:

var wakeups = thread.tempWakeups || [];

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:408: }
If we get to the end of the list of wakeups and wakeups[0].ts is still <
nextSlice.start, should we process it, or skip the processing?  (or, is that
just not possible?)

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:417: {}, midDuration));
Instead of midDuration, can we do:

wakeup ? wakeup.ts - prevSlice.end : midDuration

Then we can drop the if above.

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:573: this.wakeups_.push({ts: ts, tid: pid, fromTid:
fromPid});
Is it tid or pid? If it's tid, can we change the function signature to use tid
and fromTid? Confusing to see pid switch to tid.

[jmg, 2013-02-15 05:53:50]

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js
File src/linux_perf_importer.js (right):

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:364: if (!thread.tempWakeups)
Will do (for all of these).

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:408: }
The sched_wakeup events are interspersed with the sched_switch events from which
the existing slices are created.  A trace that ends with some number of
sched_wakeup events that aren't followed by sched_switch events doesn't seem
especially interesting to me.  It's possible, just not very useful - if the
tracer really cared about the very tail end of the trace then they probably
should have just recorded the trace longer.

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:417: {}, midDuration));
Doing that seems a little harder to read to me.  I'd prefer to leave the if
block.

https://codereview.appspot.com/7303094/diff/1/src/linux_perf_importer.js#newc...
src/linux_perf_importer.js:573: this.wakeups_.push({ts: ts, tid: pid, fromTid:
fromPid});
Welcome to the wonder world of the Linux kernel.  In the kernel each thread is
identified by a pid.  Because trace-viewer refers to thread IDs as "tid" this
sort of confusing transition needs to live somewhere.  I'll add a comment
explaining this.

[dsinclair, 2013-02-16 00:11:23]

lgtm

[nduca, 2013-02-16 00:26:08]

neat, lgtm. Thank you jamie!