Closed Bug 1508837 Opened Last year Closed 11 months ago
TTI marker incorrectly uses the User
https://searchfox.org/mozilla-central/rev/55895c49f55073d82d977cb74ec1d3a71ae4b25f/dom/base/nsDOMNavigationTiming.cpp#368 The payload for UserTimingMarkerPayload should be used for the UserTiming API for performance.mark() and performance.measure(). It will be incorrectly categorized in the profiler front-end using this payload.
Assignee: nobody → rjesup
Status: NEW → ASSIGNED
(In reply to Randell Jesup [:jesup] from comment #1) > Created attachment 9031710 [details] [diff] [review] > Add TextMarker payloads for simple Profiler Markers We do handle some "text only" markers pre-existing perf.html, but want to move away from them, and we certainly don't want to add more of them as they're hard to parse and easy to break. We definitely want to use structured payloads. Is there a compelling reason to use a text marker her? (basing my comment on the output you mentioned in bug 1514235 comment 8).
I agree with Julien; I'd be much happier with a patch that gave these markers a payload with a proper data object, with a "foregroundTab" field and maybe a "elapsedTimeSinceNavigationStart" field. The marker names seem fine.
added some alternate constructors
Attachment #9034909 - Flags: review?(mstange)
Comment on attachment 9034909 [details] [diff] [review] Add TextMarker payloads for simple Profiler Markers Review of attachment 9034909 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay. This looks good. I reconsidered my opinion on printf-style markers and I'm not opposed to them any more, at least not as much as I was before. They allow easily adding a one-off marker during debugging, they automatically format the data in the way you'd like to see them in perf.html, new markers they don't require any changes to perf.html, and having individual numbers in separate data fields is only really useful if you have some code that automatically consumes those fields, and we don't have such code at the moment.
Attachment #9034909 - Flags: review?(mstange) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c0f053802584 Add TextMarker payloads for simple Profiler Markers r=mstange
You need to log in before you can comment on or make changes to this bug.