Headless mode showing only about 40 FPS

[Pekka Paalanen]

On Fri, 31 Jul 2020 22:54:00 +0000
"Singh, Satyeshwar" <satyeshwar.singh at intel.com> wrote:

> Hey guys,
> I have been noticing that Wayland apps like simple-egl only get about
> 40 FPS in Headless mode. I followed the logic and it seems to me that
> this is inevitable in the current code (unless of course I am
> miscalculating 😉). Here’s the flow of events:
> 
>   1.  headless_output_repaint is called and sets a timer by doing
> this:
> 
> wl_event_source_timer_update(output->finish_frame_timer, 16);
> 
>   1.  The above timer when fired will call finish_frame_handler.
>   2.  finish_frame_handler will call weston_output_finish_frame.
>   3.  weston_output_finish_frame’s logic is such that it adds 16.66
> (refresh to the current time stamp), then subtracts repaint_msec
> which is hardcoded to 7 ms and finally subtracts the current time
> from it. This will invariably lead to 9 ms (rounded down from 9.66)
> as the next repaint timer to fire and start a fresh repaint cycle
> which will again wait for 16 ms before calling finish_frame_handler.
> 
> This means that there is always a gap of 16 + 9 = 25 ms. Sure enough,
> dividing 1000 / 25 gives us 40 FPS which is what I am getting. Please
> let me know if I went wrong anywhere in my logic above.
> 

Hi,

yes, that seems correct. That's a good find, I don't remember anyone
paying attention to the headless timings before.

> Now, I feel like there are two ways to fix this problem:
> 
>   1.  The easy way is to just change the code in
> headless_output_repaint
> 
> from:
> 
> wl_event_source_timer_update(output->finish_frame_timer, 16);
> 
> to:
> 
> wl_event_source_timer_update(output->finish_frame_timer, 7);
> 
> We know that the logic in weston_output_finish_frame arms the timer
> for 9 ms which will ultimately call the next repaint cycle. So when
> we get down to headless_output_repaint, we only wait for another 7 ms
> to get to a total of 16. This works for me but it looks weird when we
> are setting the timer to 7 ms which would left someone wondering what
> this is all about.
> 

Actually we don't know it's 9 ms. repaint-window is a configurable
value, so it needs to be consulted for computing the right delay.
However, this will not give an accurate frame rate either, because we
have two delays both truncated to milliseconds.

> 
>   1.  Another way to fix the problem is to have
> weston_output_finish_frame arm a timer instantaneously rather than
> for 9 ms later. In fact, I see that there is already code in the
> beginning of that function that looks like this:
> 
>     if (!stamp) {
> 
>         output->next_repaint = now;
> 
>         goto out;
> 
>     }
> 
> Which means that from the caller (finish_frame_handler) we can just
> call with NULL for stamp and the timer will be armed immediately.
> 
> It’s just that there is an assert above it to check that stamp is not
> NULL:
> 
> assert(stamp || (presented_flags & WP_PRESENTATION_FEEDBACK_INVALID));
> 
> 
> 
> So, I was thinking that if I remove the check for stamp from this
> assert and then use NULL for stamp from the caller, then we will be
> firing the timer immediately. This also worked for me and it looks
> cleaner as well. However, it is changing a key function in
> compositor.c so I am wondering if that would be okay?

The assert is there to prevent exactly the kind of abuse you are
suggesting. Either the stamp must be valid, or feedback must be invalid
because we are actually not finishing but starting a repaint cycle
from idle (called from headless_output_start_repaint_loop()).

I think the proper solution for this is to keep the "vblank
timer" (currently named headless_output::finish_frame_timer) running
continuously, armed again immediately after it fires or even use a
repeating timer for it. Maybe it's possible to start the repeating
timer on _start_repaint_loop() and stop it if there was no update
posted, following exactly how libweston core tracks the output looping
vs. idle. That is how a hardware vblank works, too, and not by arming a
timer on repaint().

Naturally that requires a bit of rearranging of the timer handler
function as it may be called even without a repaint().

In other words, the headless backend would become more like DRM
backend: the vblank timer keeps ticking independently, and only calls
weston_output_finish_frame() if there was an update posted. This is
similar to how a KMS page flip is acknowledged by a page flip event
(vblank firing).


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20200803/35fbc1cf/attachment.sig>