After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 765579 - wayland: cache frequently accessed GtkSettings in wayland screen
wayland: cache frequently accessed GtkSettings in wayland screen
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-04-26 08:40 UTC by Christian Hergert
Modified: 2016-04-29 03:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wayland: cache frequently accessed GtkSettings in wayland screen (7.99 KB, patch)
2016-04-26 08:41 UTC, Christian Hergert
none Details | Review
wayland: cache frequently accessed GtkSettings in wayland screen (7.34 KB, patch)
2016-04-26 08:46 UTC, Christian Hergert
none Details | Review
settings: cache GtkSettings properties on read-back (2.94 KB, patch)
2016-04-27 01:28 UTC, Christian Hergert
none Details | Review
settings: Cache xsettings (7.50 KB, patch)
2016-04-27 04:01 UTC, Matthias Clasen
committed Details | Review

Description Christian Hergert 2016-04-26 08:40:58 UTC
I was surprised to see GSettings show up in the hot path of cursor blinking. It
queries things quite often and read-back is just a bit more expensive than I
think we want in this path. This patch avoids some walking of the translation
table with a hashtable and caches values to avoid the query to GSettings.
Comment 1 Christian Hergert 2016-04-26 08:41:01 UTC
Created attachment 326732 [details] [review]
wayland: cache frequently accessed GtkSettings in wayland screen

Some values get queried in GtkSettings quite frequently. One such example
is "gtk-cursor-blink" which is queried by GtkTextView on every
key-press-event. GSettings is relatively fast, but performs enough work
on a read-back that it shows up on profiles.

This is a micro-optimization no doubt, but seems worth it to make
GtkSettings performance more inline with other backends.

Before:

      SELF CUMULATIVE    FUNCTION
[   0.00%] [   0.61%]    gtk_text_view_pend_cursor_blink
[   0.00%] [   0.27%]      get_cursor_time
[   0.00%] [   0.27%]        g_object_get
[   0.00%] [   0.27%]          g_object_get_valist
[   0.00%] [   0.26%]            object_get_property
[   0.00%] [   0.26%]              gtk_settings_get_property
[   0.00%] [   0.25%]                gdk_screen_get_setting
[   0.00%] [   0.25%]                  gdk_wayland_screen_get_setting
[   0.00%] [   0.25%]                    set_value_from_entry
[   0.00%] [   0.25%]                      g_settings_get_int
[   0.00%] [   0.24%]                        g_settings_get_value
[   0.00%] [   0.22%]                          g_settings_schema_key_init
[   0.00%] [   0.01%]                          g_settings_read_from_backend
[   0.00%] [   0.01%]                          g_settings_schema_key_clear
[   0.00%] [   0.00%]                          g_variant_unref
[   0.00%] [   0.01%]                        g_variant_get_int32
[   0.00%] [   0.01%]                g_value_type_transformable
[   0.00%] [   0.00%]                g_type_check_instance_is_a
[   0.00%] [   0.01%]            g_param_spec_pool_lookup

After:

      SELF CUMULATIVE    FUNCTION
[   0.00%] [   0.19%]    gtk_text_view_pend_cursor_blink
[   0.00%] [   0.08%]      cursor_blinks
[   0.00%] [   0.04%]      get_cursor_time
[   0.00%] [   0.02%]        g_object_get
[   0.00%] [   0.02%]          g_object_get_valist
[   0.00%] [   0.02%]            object_get_property
[   0.00%] [   0.02%]              gtk_settings_get_property
[   0.00%] [   0.01%]                gdk_screen_get_setting
[   0.00%] [   0.01%]                  gdk_wayland_screen_get_setting
[   0.00%] [   0.01%]                  g_type_check_instance_is_a
[   0.00%] [   0.01%]                g_param_value_validate
[   0.00%] [   0.01%]            g_param_spec_pool_lookup
Comment 2 Christian Hergert 2016-04-26 08:46:20 UTC
Created attachment 326733 [details] [review]
wayland: cache frequently accessed GtkSettings in wayland screen

update the patch with more relevant stack traces
Comment 3 Matthias Clasen 2016-04-26 20:06:26 UTC
I think if we want to cache settings, this belongs into gtksettings.c - it already has a property_values array which is perfectly suited for this task. Just store the settings values there, and clear the cache when we get a GdkEventSetting
Comment 4 Christian Hergert 2016-04-26 20:17:10 UTC
Seems like a reasonable request.
Comment 5 Christian Hergert 2016-04-27 01:28:17 UTC
Created attachment 326815 [details] [review]
settings: cache GtkSettings properties on read-back

When reading a GtkSetting from the underlying source, cache the value to
reduce the cost of querying the backend. In many situations, such as
GSettings, this can speed up get_property calls.

Upon notification of the value changing, we reset the cached value so it
falls through to the underlying data source.

This only caches values in the GTK_SETTINGS_SOURCE_DEFAULT cause to
simplify the transition back from cached to uncached state.
Comment 6 Matthias Clasen 2016-04-27 04:01:45 UTC
Created attachment 326825 [details] [review]
settings: Cache xsettings

Instead of calling out to gdk every time an XSetting is requested,
cache the value (we already have the property_values array
anyway).
Comment 7 Matthias Clasen 2016-04-27 04:03:11 UTC
This is more what I had in mind (but not fully enough tested to commit.
Comment 8 Christian Hergert 2016-04-27 11:06:54 UTC
Seems to be working well for me.
Comment 9 Matthias Clasen 2016-04-29 03:30:20 UTC
Attachment 326825 [details] pushed as 7afc6b1 - settings: Cache xsettings