Headless mode showing only about 40 FPS

[Singh, Satyeshwar]

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.

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.


  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?

Please let me know your thoughts. Any ideas here would be appreciated.
Thanks,
Satyeshwar


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20200731/23853cbf/attachment.htm>