1548003 - WipeContainingBlock marker is invalid

Status: NEW

(Core :: Gecko Profiler, defect, P2)

Description

[Julien Wajsberg [:julienw]]

WipeContainingBlock marker is invalid: it uses the type "tracing" but has no "interval" value.

{
  "start": 31428.499575484377,
  "dur": 0,
  "name": "WipeContainingBlock: IB splits",
  "title": null,
  "data": {
    "type": "tracing",
    "category": "Layout"
  }
}

[1] https://searchfox.org/mozilla-central/rev/b756e6d00728dda4121f8278a744381d8643317a/layout/base/nsCSSFrameConstructor.cpp#11245-11246

This was lately changed in bug 1520526 but the marker itself was added in bug 1498067. I don't know if bug 1520526 changed the behavior by adding this "category", what do you think Greg?

Comment 1

[Greg Tatum [:gregtatum] (REO Fx132)]

I don't understand the intent of that marker, or the intent of the TracingKind::TRACING_EVENT.

I wonder if they meant to use AUTO_PROFILER_TRACING instead.

Comment 2

[Julien Wajsberg [:julienw]]

Hey Emilio, I think you were the one interested by this marker in the first place as you did it in bug 1498067. Could you look at it, or at least shed some light? Thanks!

Comment 3

[Emilio Cobos Álvarez (:emilio)]

The intent of that marker is to know in the profiler UI that that codepath has been hit. That causes some other work to be done asynchronously, but it's helpful to know that this is the origin of the codepath.

I checked with Markus before landing that patch and in theory an interval was not needed. The profiler UI needed a fix to show that: https://github.com/devtools-html/perf.html/pull/1351

I don't understand why a tracing marker would need an interval.

Comment 4

[Julien Wajsberg [:julienw]]

Thanks Emilio, I'll double check with Markus. I realize that I asked the exact same question in the pull request, but I don't remember if I got an answer in another channel.

In our terminology a "tracing" marker is a pair of start and end markers. Maybe the terminology needs to be clarified better. We can as easily add a "marker" that wouldn't be "tracing" and representing one point in time.