Deep-link into traces

[benshayden]

The perf-insights weather-report is great at finding traces that have interesting features, but once you open a trace, you have to find those features manually. It would be more useful if it could link to specific IRs, slices/events, processes, threads, sets of events, and time ranges (... anything else?). Such deep links would also be useful for sharing between engineers who find features manually without the help of the weather-report.

The last time I mentioned this to @natduca , IIRC, he proposed a singleton object that managed structured access to location.hash. For example, when the timeline-track-view zooms, it would say DeepLinkManager.set('timeline-track-view-zoom', zoomScale), which would add '&timeline-track-view-zoom=3.14' to location.hash. The DeepLinkManager would dispatch an event to a listener when an observed property changes, so when the timeline-track-view is created, it would say DeepLinkManager.addListener('timeline-track-view-zoom', this.onDeepLinkZoom_.bind(this)). When the trace-viewer finishes loading, the DeepLinkManager would dispatch events for properties that were already in location.hash.

@zeptonaut @philippebeaudoin

Objections? Volunteers? Thoughts?

[zeptonaut]

I really like this idea, but one part of it that I'm always stumped about is, if someone selects 5000 trace events, how do we link to them? There's a limit of 2083 characters in a URL, and thus a hard limit on the amount of data that we can actually send in it.

[benshayden]

Perhaps each method of selecting multiple events could use DeepLinkManager at the conceptual level rather than the raw EventSet level.

You can select events by drawing a box: DLM.set('select-events-in-xy-ranges', {left, right, top, bottom}) Select events by searching: DLM.set('select-events-search', 'RenderFrameImpl::did') Select events by IR: DLM.set('select-ir-associated-events', ir.id) Select events by rail-score-side-panel coverage link: DLM.set('select-events-coverage', SELECT_UNCOVERED_EVENTS)

It would require writing a bit more code than simply DLM.set('selected-events', [guid, guid, guid]), but hopefully not too much more?

[natduca]

I think the key thing here is that most deep links are going to be generated by automated analyses. E.g. we're going to be bulk processing traces and picking things with a given name, etc.

Its definitely valid that there will be deep links referring to a subset of the model that exceed the hash size limit.

Maybe the right primitive here we land up front is

EventSet.asDictIfPossible() (returns undefined if >2k ish) and EventSet.fromDict(model, dict)

By default, this'd returns the eventset catenated, but will return undefined for huge ones.

This then gives us the right interface that can then be extended down the road to handle more sophsiticated situations. Namely, if down the road, if we find specific use cases where we want procedural selection, then you'd subclass Selection, make it an event registry, and fromDict() would use our traditional {type: 'myconstructorname'} scheme that we already use. I'm not wild about implementign that latter bit now, simply because we've got other stuff thats probably more urgent and the patch for this would probably end up requiring a fair bit of nuance, but maybe we could at least agree that this would be the future direction when we get to it?

Patch series:

1. add stableId() getter to slice, asyncSlice, IR. May require passing parenet ptrs to them when they construct. try not to increase size of the event objects, instead, have them ask their parent for an id for themselves [e.g. parent.slices.indexOf(this)]
2. add EventSet.fromDict and asDict.
3. modify http://localhost:8003/perf_insights_examples/trace_viewer.html to read the hash, fromDict it, and set it on TimelineView's BrushingController's selection

[natduca]

@fdoray @philippebeaudoin fyi.

[petrcermak]

+1 to adding a DeepLinkManager (which would be available in Trace Viewer as well).

[zeptonaut]

+1 to Nat's suggestion. I think that'd cover about 99% of use cases for now, and it leaves us room for future expansion in the future

[benshayden]

@fdoray Are you still driving this?