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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

Details

Attachments

(2 files)

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.
Attached patch fix v1Splinter Review
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 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+
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.
(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.
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.

Attachment

General

Created:
Updated:
Size: