Skip to content

clutter: Avoid unnecessary relayouts in ClutterText

Yussuf Khalil requested to merge pp3345/mutter:avoid-relayouts into master

I've recently started doing some investigation on GNOME's performance. One thing I've quickly noticed are spikes in the frametimes that seem to occur at regular intervals as can be seen in the following diagram:

ah5al9grcw_2018-02-08_22.12.15

As it turns out, these spikes are caused by the /org/gnome/desktop/interface/clock-show-seconds dconf setting. Disabling it will make the spikes go away. So, how can something as simple as updating the displayed time cause such a large increase in the time required to draw a frame? The root cause lies in the function buffer_notify_text() in clutter-text.c. This function is called whenever the underlying text buffer of a ClutterText changes and schedules a rather expensive relayout of the text.

To my understanding, there is no possibility for the layout to actually change if the required height and width to fully draw the text haven't changed. Thus, this patch proposes a simple fix for the issue: first, we check whether the required size after the text change is equal to the currently allocated size, and if so, we only schedule a redraw instead of a relayout.

This gives us the following results: l8awxolu4l_2018-02-08_22.40.41

The spikes aren't completely gone, but they've become a lot smaller. In terms of numbers: the median frametime when updating the displayed clock time is down from 16.97ms to 12.97ms. The remaining large spikes seem to have a different cause and are not related to updating the clock time.

This improves performance for a fairly common case: several pieces of text that are displayed in the Shell UI stay at the same size after updates most of the time (another example is battery percentage). Even when displaying the seconds is disabled, the clock will update once a minute. For these cases, we save some CPU time and avoid dropping frames due to taking too long.

According to my testing, this change doesn't create any visual regressions, but please note that I am new to GNOME and I might be missing something important. There are several other places in clutter-text.c where relayouts are scheduled that probably could be avoided in some cases, e. g. in clutter_text_set_cursor_visible() or clutter_text_set_ellipsize(). Probably it would make sense to add a method that determines whether a redraw is sufficient and use that one everywhere in clutter-text.c instead of scheduling relayouts directly. I'll be happy to extend the patch if review doesn't reveal any issues with this approach. :-)

Merge request reports