Open catapult-bot opened 9 years ago
catapult-bot
commented
9 years ago 
In this screen shot it appears that there is no flow data for this RunTask event:
However, zooming in one more level shows that there is flow data available:
Zooming in (to some unknown degree) for each event of interest is inconvenient.
catapult-bot
commented
9 years ago 
This would be awesome. To parts to this: (1a) fixing our trace_model to actually track the relationship between flows and slices [our word for events that have start + duration and live on a thread] and (1b) figuring out the semantics so that flow and task mapping actually makes sense and (2) figuring out UX.
To 1a, our parsing of the flow event stream here has always been "just enough to draw arrows" --- we never connected up the arrows to the actual slice data model. That is, we have an interval tree of flow events. And we have slices. They don't know about each other. They could. E.g. a task could know what out-flows it spawns, and what flows point at it.
But, 1b, the way flows work, the relationship of in-flows [eg a flow step or a flow-end] is funky relative to a task. I believe that when a postTask runs, we issue the flow end event and THEN issue the start task event. That is, in the buffer: ts=0.1 flowEnd postTask(x) ts=0.11 x begins ts=0.2 x ends
Kinda ew. But kinda makes sense given how we have this all set up. But, we kinda want the slice.html have an accessor like 'in_flows'. So we gotta figure out the semantics of that. I bet there's something coherent here, right? Help needed!
For ux, one possibly useful place we could add UI is in the single_slice_sub_view.html down at the bottom: once we know where a slice comes from, we could add UI for a hyperlink that would change selection to that source slice. Etc. Dunno! More help needed. :)
catapult-bot
commented
9 years ago 
The flow events are in the slice arrays, that's how they get drawn on the timeline. The interval map is used to determine if they should draw the arrows because they're on-screen. We could put in logic that points back from a slice to the one before it if it's a flow event.
The flow event ends before the RunTask event starts. They're completely separate events as far as the model is currently concerned.
Not sure I understand what you're talking about with the single slice sub view stuff.
catapult-bot
commented
9 years ago
One thing we could do, instead of not drawing the flow events if they're narrow. We always draw them, we just drop the arrow head, so they become just a line. Then, if you zoomed in we could add in the decorations when there is enough space.
In theory they'd always show up on the timeline that way.
catapult-bot
commented
9 years ago
Turns out we already drop the arrows, just had to remove the minimum width check and it seems to be working. Tested by zooming way out on the flow simple example. Before, lines would disappear, now they stay no matter how far out I go.
catapult-bot
commented
9 years ago
Does removing the minimum width lead to too much clutter? If not then great, declare victory, go home, maybe, as long as the flow events are realistically visible.
There is still the problem that if you select a whole row of RunTask events then the flow events that get selected simultaneously (which should now be visible, because of removing the minimum flow arrow width, but that's not on canary yet) will not necessarily be the correct flow events.
That is, due to the behavior described in issue 787, the associated flow event (the immediately prior flow event on the same thread) is not necessarily on the same line, and therefore may not get selected. This may not matter as much if issue 787 is resolved since the user will be able to select a single RunTask and then left arrow to select the flow event.
When writing my simple trace parser that matches up flow events to tasks I didn't really hit any problems (see 1b, above). My script just keeps a dictionary (keyed by pid/tid) that contains the last event on each thread. When I come across an event of interest (in my case, a particular RunTask) I just look up the previous event on that thread. If its name is "MessageLoop::PostTask" and its ph is "f" then I hook them together. Easy. And it should be reliable. Roughly speaking:
previousFlowEvent = None
previousThreadEvent = recentThreadEvents.get((event["pid"], event["tid"]))
if previousThreadEvent:
if previousThreadEvent["name"] == "MessageLoop::PostTask" and previousThreadEvent["ph"] == "f":
previousFlowEvent = previousThreadEvent
As a tone-calibrating comment, telling us that this "this should be easy" has only one response: patches welcome.
There are valid reasons that we're taking our time on this.
I have made #822 for the model changes. We can discuss the UI proposal here once that bug is closed.
catapult-bot
commented
9 years ago
Well, it's easy in the context of a single-purpose parser. I didn't mean to speak to the complexity of making the change in the context of the full TraceViewer.
If a flow event was in the queue for a short period of time then it will be completely invisible unless you zoom in on exactly the right spot. This is non-obvious to a new user. Even to an experienced user it means that there is no easy way to distinguish between a flow event that is missing, and a flow event that has a very short queue_duration.
A possible fix to this is to force the display of flow events for tasks that are selected. This allows the user to click a particular task and instantly and reliably see when it was queue, or to instantly know if that information is not available.