1071183 - [timeline] support unknown markers

Status: RESOLVED WONTFIX

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Paul Rouget [:paul]]

We want to be able to send markers unknown by the frontend. They would all appear in the same line in the overview, and with the same color in the waterfall.

I will provide a patch as an example.

Comment 1

[Paul Rouget [:paul]]

Easier than a patch: remove from global.js the Paint section, and run the timeline. We want the paint markers to appear in the timeline anyway.

This will be useful to support console.time("XX") and console.timeEnd("XX") to appear as markers. Colors would be the same as console.time("YY"), the only difference would be the label in the name column.

Comment 2

[Paul Rouget [:paul]]

Attachment: WIP

Comment 3

[Paul Rouget [:paul]]

Attachment: v1

This adds an extra line. It should only appear when there's an unknown marker, but I'll do that in a followup bug.

Comment 4

[Paul Rouget [:paul]]

Attachment: v1 (better colors)

Comment 5

[Paul Rouget [:paul]]

(commit message in the patch is wrong)

Comment 6

[Paul Rouget [:paul]]

Also - because we know nothing about these markers, they can totally overlap.

M1: 1000 -> 2000
M2: 1200 -> 1800

M2 won't be visible in the overview. Using hsla colors doesn't appear to make the marker transparent.

We should address that in a follow-up (we need this to land asap to support console.time/timeEnd),

Comment 7

[Victor Porof [:vporof][:vp]]

Comment on attachment 8494441 [details] [diff] [review]
v1 (better colors)

Review of attachment 8494441 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/timeline/widgets/global.js
@@ +44,5 @@
>      stroke: "hsl(104,57%,51%)",
>      label: L10N.getStr("timeline.label.reflow")
>    },
> +  "Unknown": {
> +    group: 3,

Doesn't this add some unused space in the overview graph? That's most likely undesirable. I also don't think we need to show these, just silently ignore them.

Comment 8

[Paul Rouget [:paul]]

(In reply to Victor Porof [:vporof][:vp] from comment #7)
> Comment on attachment 8494441 [details] [diff] [review]
> v1 (better colors)
> 
> Review of attachment 8494441 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/timeline/widgets/global.js
> @@ +44,5 @@
> >      stroke: "hsl(104,57%,51%)",
> >      label: L10N.getStr("timeline.label.reflow")
> >    },
> > +  "Unknown": {
> > +    group: 3,
> 
> Doesn't this add some unused space in the overview graph? That's most likely
> undesirable. I also don't think we need to show these, just silently ignore
> them.

We want a bucket for anything that we don't expect client side. If there are data coming in, we definitely want to show them. It's useful for backward compatibility, and also for anything that doesn't require its own bucket (like console.time).

We could hide it until we get an unknown event.

Would that work for you?

Comment 9

[Victor Porof [:vporof][:vp]]

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #8)
> (In reply to Victor Porof [:vporof][:vp] from comment #7)
> > Comment on attachment 8494441 [details] [diff] [review]
> > v1 (better colors)
> > 
> > Review of attachment 8494441 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: browser/devtools/timeline/widgets/global.js
> > @@ +44,5 @@
> > >      stroke: "hsl(104,57%,51%)",
> > >      label: L10N.getStr("timeline.label.reflow")
> > >    },
> > > +  "Unknown": {
> > > +    group: 3,
> > 
> > Doesn't this add some unused space in the overview graph? That's most likely
> > undesirable. I also don't think we need to show these, just silently ignore
> > them.
> 
> We want a bucket for anything that we don't expect client side. If there are
> data coming in, we definitely want to show them. It's useful for backward
> compatibility, and also for anything that doesn't require its own bucket
> (like console.time).
> 
> We could hide it until we get an unknown event.
> 
> Would that work for you?

If it's something like console.time, that deserves it's own dedicated category. Anyway, unknowns should stay unknowns. What's wrong with just logging an error in the console? I'm assuming all of this is just for the purpose of identifying bugs, not that users will be in any way interested in seeing.

Comment 10

[Paul Rouget [:paul]]

(In reply to Victor Porof [:vporof][:vp] from comment #9)
> If it's something like console.time, that deserves it's own dedicated
> category. Anyway, unknowns should stay unknowns. What's wrong with just
> logging an error in the console? I'm assuming all of this is just for the
> purpose of identifying bugs, not that users will be in any way interested in
> seeing.

If you're a B2G developer and want to trace, let's say, the time it takes to compute the DLBI. You will add your own marker, and they won't appear in the timeline.

Comment 11

[Paul Rouget [:paul]]

Console markers look like this: "ConsoleTime:XXX", where XXX is the label used by the user. So the timeline UI, instead of doing a simple check like `marker.name == blueprint.name`, will have to do something like `blueprint.name.startsWidth(marker.name)`, or the blueprint could expose regex instead of plain strings.

So that means a B2G developer could simply add a marker starting with "ConsoleTime:...", and its marker will end-up in the console.time bucket. So no need for a generic marker bucket.

What do you think?

Comment 12

[Paul Rouget [:paul]]

No. I found a much better way to do that.

ConsoleTime:* markers will be renamed ConsoleTime. And the name of the marker will be added to a details property. See bug 1050502.

Comment 13

[David Lawrence [:dkl]]

Sorry for the spam. Moving bugs to Firefox :: Developer Tools: Performance Tools (Profiler/Timeline).

dkl

Comment 14

[alexmeen]

https://inshotproapp.com/ Inshot Pro is a powerful video editing & Photo editing app for your Android device.