requestAnimationFrame is called ~1500 times per second

[paulrouget]

Which is a little bit too much :)

<script>
  var count = 0;
  function step() {
    count++;
    requestAnimationFrame(step);
  }

  setInterval(function() {
    document.body.textContent = count + "FPS";
    count = 0;
  }, 1000);

  step();
</script>

[glennw]

Yep - I ran across this yesterday. The callback is triggered whenever the compositor runs, which is as fast as possible when animations are running (which is also wrong). Instead, it should only trigger the callback after an entire paint completes for the relevant pipeline. This is a little bit tricky, since it needs to check the epoch for each layer that is painted for the pipeline in question.

[Yoric]

@glennw I'd like to work on this, if you can give me a hand.

I am trying to understand to follow the chain that triggers the callback. As I understand, this goes through ConstellationControlMsg::TickAllAnimations, which we want to send less often. In turn this goes through LayoutControlMsg::TickAnimations, ConstellationControMsg::TickAnimations and to change_running_animations_state. Is this where we want to throttle the callback or should I climb the chain a little bit further?

[paulrouget]

The callback is triggered whenever the compositor runs, which is as fast as possible when animations are running (which is also wrong).

In my case, there was no animation.

Instead, it should only trigger the callback after an entire paint completes for the relevant pipeline.

What is wrong with keeping the same strategy (RFA when the compositor runs), but driving the compositor with vsync? Isn't that how it's supposed to work?

[glennw]

@paulrouget If we drive it with vsync we could get extra callbacks when the repainting is taking longer than a vsync period, and we should also support running in non-vsync mode correctly, I think.

@Yoric Great! I think what we want to do is throttle the callback in process_animations which is called each time the compositor draws a frame - in such a way that it only ticks animations if there isn't an outstanding paint being waited on for the relevant pipeline.

@pcwalton Does that sound correct to you?

[Yoric]

According to my trusty Gecko Compositor hacker @nical, we should really drive it with vsync. Preventing double-callbacks shouldn't be too hard, I guess.

@glennw Why do we want to support non-vsync mode, btw?

[nical]

Non-vsync mode is useful if you want to benchmark layout or rendering performance, but it's important for frame uniformity to drive RaF from vsyncs in the general case. Better yet, detecting that we are not keeping up with vsync and skipping every other vsyncs when that happens to keep frames uniforms (or when we want to save energy) is pretty useful.
It should not be hard to avoid the extra callback problem (you more or less just need to ignore incoming vsync notifications when you have a vsync callback that hasn't completed yet).

[glennw]

@Yoric Yep, exactly what nical said. I should have been clearer in saying that driving it from vsync is what we want, we just need to take account of those edge cases (vsync disabled, and painting not keeping up with vsync) and make sure they work too.

[Yoric]

@glennw Oh, so we have vsync support already?

[glennw]

@Yoric Only in so much as (I think) we set the default swap interval to vsync - it may even just be that it defaults to vsync on my linux machine.

[Yoric]

I'm a bit lost. If process_animations is called 1500 times per second, then so are process_pending_scroll_events and window.present() (which I understand is window flipping: documentation needed), so I guess we need to throttle all of these to vsync. Are you telling me that this is probably already the case at least on Linux?

If so, your earlier comment is a fallback for platforms that do not have vsync, right? I'm not sure I understand why finding out if the pipeline is waiting for a paint is related to the issue at hand (unless that's the vsync?) but I can certainly investigate this. Is this what does_layer_have_outstanding_paint_messages does or should I look on the other side of the pipeline?

[nical]

Yoric already knows about this document, but for the record, here is the design doc of the vsync stuff in gecko: https://dxr.mozilla.org/mozilla-central/source/gfx/doc/Silk.md

[pcwalton]

If window.present is being called over 60 FPS then we need to fix that problem at the windowing layer. That has to be fixed before anything else will work.

[paulrouget]

Now we are more advanced in browser.html support, this ends up being a problem.

Many animations in browser.html are RAF driven, and having ~700 calls per seconds is really impacting the performance.

[paulrouget]

If window.present is being called over 60 FPS then we need to fix that problem at the windowing layer. That has to be fixed before anything else will work.

It's called 60 FPS.

[glennw]

I think this will be a lot easier to solve with the WR design than the existing layers / painting design - I'll look into once I've cleared out my current bug list.

[glennw]

I pushed glennw@be3213e to the WR servo branch. In my test case, with vsync enabled (which is the default), I am getting rAF callbacks at exactly 60 fps, synced with the presentation interval.

I've also opened a PR to submit this to upstream servo here - #9401 - however even once this lands upstream there may be issues with non-WR painting backend in some cases.

[paulrouget]

@glennw on osx, I'm still getting ~500 calls.

[glennw]

It sounds like maybe vsync isn't working? Is there any way to verify if an application is running with vsync enabled?

[paulrouget]

I added to comment 0 the script I use to count the FPS.

[glennw]

I tried that test case on Linux and it's reporting 60 fps (well 61-62 but close enough for this test).

[paulrouget]

vsync is enabled on mac (at least, glutin's WindowBuilder::with_vsync is called).

[paulrouget]

Actually - I was wrong. It runs at 60 FPS.
Sorry sorry. I must have run a release build and compiled a debug build.

[paulrouget]

I figured it out. Using the -b option makes the FPS go crazy. Not using -b, it goes at 60 FPS.

[paulrouget]

#9431

[pcwalton]

@paulrouget Can we close this in favor of #9431?

[paulrouget]

@pcwalton This issue will get fixed once #9244 lands.

[paulrouget]

Sorry, I meant when #9401 lands.

[paulrouget]

(not a browserhtml-P1 anymore as it has been fixed in webrender)