Closed
Bug 753630
Opened 13 years ago
Closed 12 years ago
Beef up animation detection heuristics for code discarding
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
People
(Reporter: bhackett1024, Assigned: bhackett1024)
References
Details
(Whiteboard: [js:p1:fx16][k9o:p1:fx16])
Attachments
(2 files)
The heuristics for deciding whether to discard jitcode on GC are currently tied to calls to window.requestAnimationFrame. Per bug 750834 comment 20 most websites which play animations don't use this API, but rather use setTimeout to control animation timing. The heuristics should be improved to capture these websites as well, while retaining a low false positive rate (we don't want to treat websites as playing animations if they actually aren't, as then extra analysis info and jitcode may be kept around longer than necessary).
Can we enable eviction when no JS has run for one second?
Comment 2•13 years ago
|
||
Alternatively, can we somehow set a flag in a compartment on invocation of setTimeout with an interval of less than, say, 100ms?
Updated•13 years ago
|
blocking-kilimanjaro: --- → +
Whiteboard: [k9o:p1:fx15]
Comment 3•13 years ago
|
||
It might be good to coordinate with the heuristic in bug 712478.
Comment 4•13 years ago
|
||
Just as a quick data point: for the attached setTimeout-based version of the Spinning Balls benchmark, the animation heuristics already seem to be working. In no GC do I get Discard Code totals longer than 0.3ms.
Assignee | ||
Comment 5•13 years ago
|
||
(In reply to Till Schneidereit [:till] from comment #4)
> Created attachment 624526 [details]
> Spinning Balls benchmark, modified to be setTimeout-based
>
> Just as a quick data point: for the attached setTimeout-based version of the
> Spinning Balls benchmark, the animation heuristics already seem to be
> working. In no GC do I get Discard Code totals longer than 0.3ms.
Code discarding only really affects things when the animation/website runs a lot of code which would normally need to be reanalyzed and recompiled after each GC. The spinning balls benchmark is a reskinned v8-splay, which has a very tiny amount of code but allocates a ton of objects (so that scanning for singleton objects when preserving code is expensive).
Comment 6•13 years ago
|
||
> Code discarding only really affects things when the animation/website runs a
> lot of code which would normally need to be reanalyzed and recompiled after
> each GC. The spinning balls benchmark is a reskinned v8-splay, which has a
> very tiny amount of code but allocates a ton of objects (so that scanning
> for singleton objects when preserving code is expensive).
I just started figuring out as much by profiling what the vm is doing. Thanks for the explanation - that saves me a lot of further digging.
Updated•13 years ago
|
Whiteboard: [k9o:p1:fx15] → [k9o:p1:fx15][js:p1:fx15]
Updated•12 years ago
|
Whiteboard: [k9o:p1:fx15][js:p1:fx15] → [k9o:p1:fx15][js:p1:fx16]
Updated•12 years ago
|
Whiteboard: [k9o:p1:fx15][js:p1:fx16] → [js:p1:fx16][k9o:p1:fx15]
Updated•12 years ago
|
Whiteboard: [js:p1:fx16][k9o:p1:fx15] → [js:p1:fx16][k9o:p1:fx16]
Comment 7•12 years ago
|
||
Brian's been meaning to take this--he says he'll be able to get onto it by the end of next week.
Assignee: general → bhackett1024
Assignee | ||
Comment 8•12 years ago
|
||
Apologies for the delay here, it's taken some time to find a heuristic that I liked. I tried out the suggestions in comment 1 and 2, but these and similar things seem to trigger on many websites which don't use animations. Now, keeping code around on these sites might not be bad but I'm not aware of any evidence that recompilation costs hurt outside of animations, and would rather not overreach here (avoiding code discarding unnecessarily will increase memory usage).
This heuristic just treats invalidation of all or part of a canvas element as animation activity, in the same way as calling mozRequestAnimationFrame. This will only trip on pages that are updating a canvas element, and doesn't seem to trip on pages that are in the background. Additionally, there are already throttling mechanisms to avoid excessive canvas invalidations, so the overhead of notifying here (mainly a gettimeofday) should be small.
Attachment #637327 -
Flags: review?(dmandelin)
Comment on attachment 637327 [details] [diff] [review]
patch
Looks good. Just put {} around NotifyAnimationActivity.
There will be some kinds of animations not caught, because not everything uses canvas, but this will help.
Attachment #637327 -
Flags: feedback+
Comment 10•12 years ago
|
||
Comment on attachment 637327 [details] [diff] [review]
patch
Review of attachment 637327 [details] [diff] [review]:
-----------------------------------------------------------------
Beautifully simple.
Attachment #637327 -
Flags: review?(dmandelin) → review+
Assignee | ||
Comment 11•12 years ago
|
||
Comment 12•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in
before you can comment on or make changes to this bug.
Description
•