Closed
Bug 1077438
Opened 10 years ago
Closed 10 years ago
Timeline actor should manage Framerate and Memory actors
Categories
(DevTools :: Performance Tools (Profiler/Timeline), defect)
DevTools
Performance Tools (Profiler/Timeline)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 35
People
(Reporter: jsantell, Assigned: vporof)
References
Details
Attachments
(1 file, 1 obsolete file)
8.51 KB,
patch
|
jsantell
:
review+
|
Details | Diff | Splinter Review |
Just like the memory actor. This syncs all of the front-ends timeline data (memory, fps, markers), and if we land this for Gecko 35, then we will seriously reduce the amount of feature detection needed to support older Geckos.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Assignee | ||
Updated•10 years ago
|
Blocks: perf-tool-v2
Assignee | ||
Comment 3•10 years ago
|
||
Memory actor hasn't landed yet, so need to do both things here.
Summary: Timeline actor should manage Framerate Actor → Timeline actor should manage Framerate and Memory actors
Comment 4•10 years ago
|
||
(In reply to Victor Porof [:vporof][:vp] from comment #3) > Memory actor hasn't landed yet, so need to do both things here. What part of the memory actor are you referring to? The actual actor has existed for a long time; the allocations methods more recently, but it has definitely landed.
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Nick Fitzgerald [:fitzgen] from comment #4) > (In reply to Victor Porof [:vporof][:vp] from comment #3) > > Memory actor hasn't landed yet, so need to do both things here. > > What part of the memory actor are you referring to? The actual actor has > existed for a long time; the allocations methods more recently, but it has > definitely landed. Damn it, I accidentally some words. I meant "the memory actor usage from the timeline actor" hasn't landed yet, specifically bug 1069421.
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8503035 -
Flags: review?(jsantell)
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8503035 -
Attachment is obsolete: true
Attachment #8503035 -
Flags: review?(jsantell)
Attachment #8503036 -
Flags: review?(jsantell)
Reporter | ||
Comment 8•10 years ago
|
||
Comment on attachment 8503036 [details] [diff] [review] v2 Review of attachment 8503036 [details] [diff] [review]: ----------------------------------------------------------------- R+, but wonder if it'd be better to have one event for all of this data, rather than 3 separate ones, like "data" that contains memory, framerate and marker data, so the front only has to listen to one event, and they'd already be synced, which would alleviate any invalid timing issues that we were discussing before. Just a nit, I'm just happy to get this in for Fx35 regardless!
Attachment #8503036 -
Flags: review?(jsantell) → review+
Assignee | ||
Comment 9•10 years ago
|
||
Emitting everything in a single event is definitely more efficient, but I really don't want to deal with backwards compatibility issues anymore. Since the current "markers" event emits arrays, it's not natural at all to augment that with other data.
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/661453731c9a
Whiteboard: [fixed-in-fx-team]
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/661453731c9a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
Comment 12•10 years ago
|
||
Setting qe-verify- for this patch, since it already has automated test coverage. Victor, if you think there's something manual QA should look at here, please flip the flag.
Flags: qe-verify-
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•