Refresh driver should use REPEATING_PRECISE_CAN_SKIP timer by default

REOPENED
Unassigned

Status

()

defect
REOPENED
8 years ago
2 months ago

People

(Reporter: cjones, Unassigned)

Tracking

unspecified
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

A SLACK timer has the effect of causing animations to drift away from 60fps depending on how long the repaint callback takes.  For example, a callback that takes 16ms will get 30fps, and one that takes 8ms will get 40fps, even though both can drive 60fps animations.  The only way to achieve full 60fps is with a callback that takes 0ms.  (And of course throughout I'm assuming perfect timers.)

We should try as hard as we can to hit 60fps.  The simplest way of doing that in the current code is to switch to REPEATING_PRECISE_CAN_SKIP wholesale, not just for requestanimationframe callbacks.

In b2g's home screen, we saw a nice qualitative perf improvement when flipping the pref to always use CAN_SKIP.  However, the home screen uses RAF with webgl and a simple CSS animation, which are supposed to flip on CAN_SKIP already.  So now I'm not so sure what was happening with b2g.  mwu did you get any quantitative data from the pref flip?

However, I think we should make this change anyway.
Assignee: nobody → jones.chris.g
Well we had the FPS counter on. That doesn't give the best data but it clearly went from hovering around 35 FPS to around 55 FPS.
Comment on attachment 581561 [details] [diff] [review]
Use REPEATING_TIMER_PRECISE_CAN_SKIP instead of SLACK to try to stay closer to 60fps rendering

> However, the home screen uses RAF with webgl and a simple CSS animation, which
> are supposed to flip on CAN_SKIP already.

CSS stuff doesn't.  RAF does.  Can you fix the comment accordingly?

r=me.
Attachment #581561 - Flags: review?(bzbarsky) → review+
Speaking of which, can you just write requestAnimationFrame?  RAF is the Royal Air Force.
(In reply to Boris Zbarsky (:bz) from comment #3)
> Comment on attachment 581561 [details] [diff] [review]
> Use REPEATING_TIMER_PRECISE_CAN_SKIP instead of SLACK to try to stay closer
> to 60fps rendering
> 
> > However, the home screen uses RAF with webgl and a simple CSS animation, which
> > are supposed to flip on CAN_SKIP already.
> 
> CSS stuff doesn't.  RAF does.  Can you fix the comment accordingly?
> 

Oh hm.  I asked and was told that it does.  Will fix.

(In reply to David Baron [:dbaron] from comment #4)
> Speaking of which, can you just write requestAnimationFrame?  RAF is the
> Royal Air Force.

OK, but I could also say that FPS means "first person shooter".  If requestAnimationFrame is abbreviated, is "raf" better?  (My fingers are lazy.)
Similar results for tdhtml_paint and tdhtml_nochrome_paint.  I don't have any guesses about what's causing this, but I suspect some timing is being altered a bit.

http://perf.snarkfest.net/compare-talos/index.html?oldRevs=30e270816255,221eccfa6a3f,ceffbd680e2c,0c46aac6a23f,96a644cb3158,9fae755f5103,58a278555680&newRev=20521568a931&tests=a11y,tdhtml,tdhtml_nochrome,a11y_paint,tdhtml_paint,tdhtml_nochrome_paint,tp4,tp4_memset,tp4_pbytes,tp4_rss,tp4_shutdown,tp4_xres,tp5,tp5_memset,tp5_pbytes,tp5_rss,tp5_shutdown,tp5_xres,tp5_paint,tp5_memset_paint,tp5_pbytes_paint,tp5_rss_paint,tp5_shutdown_paint,tp5_xres_paint,dromaeo_basics,dromaeo_css,dromaeo_dom,dromaeo_jslib,dromaeo_sunspider,dromaeo_v8,tsspider,tsspider_nochrome,tsspider_paint,tsspider_nochrome_paint,tgfx,tgfx_nochrome,tgfx_paint,tgfx_nochrome_paint,tscroll,tsvg,tsvg_opacity,ts,ts_paint,ts_cold,ts_cold_generated_max,ts_cold_generated_max_shutdown,ts_cold_generated_med,ts_cold_generated_med_shutdown,ts_cold_generated_min,ts_cold_generated_min_shutdown,ts_cold_shutdown,ts_places_generated_max,ts_places_generated_max_shutdown,ts_places_generated_med,ts_places_generated_med_shutdown,ts_places_generated_min,ts_places_generated_min_shutdown,ts_shutdown,twinopen,tpaint&submit=true

I'm not going to have time to investigate this deeply for a while :(.  Un-assigning myself.  Someone else, please feel free to steal!
Assignee: jones.chris.g → nobody
https://hg.mozilla.org/mozilla-central/rev/0dbdc8243364
Assignee: nobody → jones.chris.g
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Ugh, sorry, I typo'd the first bug number in the comment.  The b2g "preview landing" was bug 717532.  Talos issues here still to be sorted out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whups, mid-aired.
Assignee: jones.chris.g → nobody
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.