Closed
Bug 1175228
Opened 9 years ago
Closed 9 years ago
Don't call profiler_tracing for scripts (requestAnimationFrame) if we don't have any work to do
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
Details
Attachments
(2 files)
3.42 KB,
patch
|
Details | Diff | Splinter Review | |
1.54 KB,
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
Right now, we call...
> profiler_tracing("Paint", "Scripts", TRACING_INTERVAL_START);
...around the chunk of code that triggers requestAnimationFrame callbacks, even if we don't actually have any callbacks.
So it seems like we may end up getting a blip on profiles when really there aren't any callbacks.
We should avoid making this call if we don't have any callbacks, to avoid spurious profiler blips.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Here's a "diff -w" version of the fix. I'm just adding an emptiness check around the profiler_tracking call and the loop. (The loop is a no-op if the array is empty.) We could do this as an early-return, too. But since this function is a bit complex & has no early-returns yet (and it's conceivable that someone might add someday add some final cleanup code at the end of the function), I figured it'd be better to just add an "if" check rather than an explicit return.
Attachment #8623197 -
Flags: review?(bgirard)
Comment 3•9 years ago
|
||
Comment on attachment 8623197 [details] [diff] [review] fix v1, "diff -w" version Review of attachment 8623197 [details] [diff] [review]: ----------------------------------------------------------------- + on avoiding the early return. Looks good.
Attachment #8623197 -
Flags: review?(bgirard) → review+
Comment 4•9 years ago
|
||
CC'ing :tromey FYI. You're still working on the timeline right? This will change the definition of the devtools timeline markers but AFAIK this will make them strictly better.
Comment 6•9 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #4) > CC'ing :tromey FYI. You're still working on the timeline right? This will > change the definition of the devtools timeline markers but AFAIK this will > make them strictly better. Thanks for the heads up. I agree, this is better.
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f049fcbdfe1e
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•