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 780555 - reduce resident memory footprint of gsd plugins
reduce resident memory footprint of gsd plugins
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: plugins
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2017-03-26 05:27 UTC by Christian Hergert
Modified: 2017-04-03 07:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Avoid loading Adwaita into memory by providing an alternate theme (2.99 KB, patch)
2017-03-27 07:31 UTC, Christian Hergert
none Details | Review
use GTK_THEME to avoid parsing of Adwaita theme (3.11 KB, patch)
2017-03-27 07:37 UTC, Christian Hergert
none Details | Review
Avoid parsing Adwaita into non-visual processes (6.05 KB, patch)
2017-03-27 08:09 UTC, Christian Hergert
needs-work Details | Review
gtk: avoid loading Adwaita CSS theme into memory (5.17 KB, patch)
2017-03-27 09:42 UTC, Christian Hergert
reviewed Details | Review
common: avoid loading Adwaita CSS theme into memory (6.55 KB, patch)
2017-03-27 21:24 UTC, Christian Hergert
none Details | Review
housekeeping: Don't init GTK+ (696 bytes, patch)
2017-03-28 09:55 UTC, Bastien Nocera
committed Details | Review
print-notifications: Don't init GTK+ (751 bytes, patch)
2017-03-28 09:55 UTC, Bastien Nocera
committed Details | Review
wacom: Don't init GTK+ (640 bytes, patch)
2017-03-28 09:56 UTC, Bastien Nocera
committed Details | Review
common: avoid loading Adwaita CSS theme into memory (5.98 KB, patch)
2017-03-28 10:02 UTC, Bastien Nocera
committed Details | Review

Description Christian Hergert 2017-03-26 05:27:50 UTC
Each of the gsd plugins seem to hover around 18mb on my system (F26). That seems awfully high for things like the housekeeping plugin.

Taking a quick look over things, I think this is related to some of the plugins unnecessarily linking against, and initializing, gtk.

I haven't verified this yet, but I suspect that simply initializing Gtk would cause the GtkSettings to load and cascade into CSS parsing/loading of Adwaita.
Comment 1 Christian Hergert 2017-03-26 05:34:04 UTC
Making a test theme with empty CSS and forcing that with GTK_THEME=simple reduces the overhead by about 3mb for my test-case.

So probably some other places to trim some fat too.
Comment 2 Bastien Nocera 2017-03-26 15:57:23 UTC
Which of the plugins do you think are linked to GTK+ but shouldn't be? I'm pretty sure they match the actual GTK+ usage.

We might want to remove GTK+ usage from some of the plugins though, but that would require looking at each change specifically, rather than in a generic bug like this one.
Comment 3 Christian Hergert 2017-03-26 21:53:04 UTC
I only looked into one, which I expected (having previously read the code) to not need gtk whatsoever: housekeeping.

Of course, some of them will need display access/etc, especially on x11. But maybe we can teach Gtk to be more lazy about parsing CSS so we don't waste 3*21*2 (3mb for CSS, 21 gsd processes, gdm + user) = 126mb of unnecessary resident memory overhead.
Comment 4 Christian Hergert 2017-03-27 07:31:05 UTC
Created attachment 348770 [details] [review]
Avoid loading Adwaita into memory by providing an alternate theme

I still have some 30 modules left to build in my jhbuild to test this, but here is a patch for a rather simple way to work around the Gtk issue.

The important detail that makes g_setenv() the most attractive route is that it
can be set before GtkSettings loads. The loading of GtkSettings is what cascades
into the Adwaita theme parsing/loading.

The resource embedding using the /org/gtk prefix seems to be the easiest route
because there is no way to setup load paths for Gtk for resources.

My test case to verify this works with a simple gtk program is as follows. It
reliably shaves 3MB off the process at startup. With the power of multiplication,
this should get us an extra 150MB or so back on our system.

int main (int argc, char *argv[]) {
  if (argc > 1)
    g_setenv ("GTK_THEME", "Disabled", TRUE);
  g_resources_register (gsd_get_resource ());
  gtk_init (&argc, &argv);
  gtk_main ();
  return 0;
}
Comment 5 Christian Hergert 2017-03-27 07:37:43 UTC
Created attachment 348771 [details] [review]
use GTK_THEME to avoid parsing of Adwaita theme

It looks like only 11 of the daemons link against gtk. So we're looking more at a
savings of around 66mb.
Comment 6 Christian Hergert 2017-03-27 08:09:55 UTC
Created attachment 348774 [details] [review]
Avoid parsing Adwaita into non-visual processes

Updated patch that builds.

What I still find very strange, is that:

 - /usr/libexec/gsd-housekeeping (user)  ~ 24mb
 - /opt/gnome/libexec/gsd-housekeeping   ~ 20.5mb
 - /usr/libexec/gsd-housekeeping (gdm)   ~ 15mb

Why is the gdm version of housekeeping 9mb less? (Also, is it necessary to run in
gdm mode at all?)
Comment 7 Bastien Nocera 2017-03-27 08:27:12 UTC
(In reply to Christian Hergert from comment #6)
> Created attachment 348774 [details] [review] [review]
> Avoid parsing Adwaita into non-visual processes
> 
> Updated patch that builds.
> 
> What I still find very strange, is that:
> 
>  - /usr/libexec/gsd-housekeeping (user)  ~ 24mb
>  - /opt/gnome/libexec/gsd-housekeeping   ~ 20.5mb
>  - /usr/libexec/gsd-housekeeping (gdm)   ~ 15mb
> 
> Why is the gdm version of housekeeping 9mb less? (Also, is it necessary to
> run in
> gdm mode at all?)

Because it's a stub. Look at daemon-skeleton-gtk.h. It shouldn't even be running under gdm, but gdm doesn't seem to have its own .session file anymore, which means it must use the same session setup as a normal session, hence the stub daemons.
Comment 8 Bastien Nocera 2017-03-27 08:31:22 UTC
Review of attachment 348774 [details] [review]:

We also need to verify which plugins use GTK+, and whether they actually show any dialogues on screen.
For some of them, we might already be able to remove the GTK+ linkage. We'll also want to look at indirect dependencies (do we really need to use ca_gtk_* ?)

::: plugins/common/Makefile.am.gresources
@@ +15,3 @@
+#	EXTRA_DIST
+#
+# Author: Christian Hergert <christian@hergert.me>

You can also remove the whole header.

@@ +18,3 @@
+
+# Basic sanity checks
+$(if $(GLIB_COMPILE_RESOURCES),,$(error Need to define GLIB_COMPILE_RESOURCES))

You can also remove the sanity checks.

@@ +47,3 @@
+	&& (cmp -s xgen-gr.h $(glib_resources_h) || cp -f xgen-gr.h $(glib_resources_h)) \
+	&& rm -f xgen-gr.h \
+	&& echo timestamp > $(@F)

Any way to avoid use a stamp file?
Comment 9 Christian Hergert 2017-03-27 09:36:58 UTC
Rookie mistake, I was looking at RES (Resident) without subtracting SHR (Shared).

So that really brings things to:

    RES   SHR
    23mb  17mb = 6mb
to  21mb  17mb = 4mb

So that isn't so bad in general, but also a ~30% reduction.
Comment 10 Christian Hergert 2017-03-27 09:39:47 UTC
Review of attachment 348774 [details] [review]:

::: plugins/common/Makefile.am.gresources
@@ +47,3 @@
+	&& (cmp -s xgen-gr.h $(glib_resources_h) || cp -f xgen-gr.h $(glib_resources_h)) \
+	&& rm -f xgen-gr.h \
+	&& echo timestamp > $(@F)

I had very back luck with parallel builds when not using a stamp file, due to multi-file dependencies with make.
Comment 11 Christian Hergert 2017-03-27 09:42:03 UTC
Created attachment 348786 [details] [review]
gtk: avoid loading Adwaita CSS theme into memory

The various Gtk programs are not dependent on any specific theme being
loaded. Therefore, the parsing the Adwaita CSS theme (which is quite a
detailed theme) is unnecessary and a few MB of overhead to each gsd
subprocess.

By setting the GTK_THEME environment variable in main() and providing an
alternate CSS file (which is empty), we can force Gtk to never load the
default theme, but instead our empty theme. This is important as otherwise
GtkSettings can force-load Adwaita upon first use, and that fragments the
heap.
Comment 12 Bastien Nocera 2017-03-27 16:14:09 UTC
(In reply to Christian Hergert from comment #9)
> Rookie mistake, I was looking at RES (Resident) without subtracting SHR
> (Shared).

Teehee. It happens to the best of us ;)
Comment 13 Bastien Nocera 2017-03-27 16:15:41 UTC
Review of attachment 348786 [details] [review]:

> gtk: avoid loading Adwaita CSS theme into memory

s/gtk/common/

I'll need to review all the GTK+ usage in the various daemons before committing this. Looks good in principle.
Comment 14 Bastien Nocera 2017-03-27 16:50:25 UTC
Plugins that currently use the GTK+ skeleton, and reasoning:
- a11y-keyboard:
  Uses GTK+ and X11 to access a11y features, doesn't need Adwaita
- clipboard:
  Uses X11 as well, doesn't need Adwaita
- color:
  Uses libcanberra, but should use a notification instead (see bug 780603)
- housekeeping:
  Doesn't use anything GTK+
- keyboard:
  Uses X11 as well
- media-keys:
  Use GtkSettings, and a bunch of other GTK+ helpers. Would setting the env as we do in that hack override the env for launched apps though? gtk_show_uri() in particular (our uses of g_app_info_launch() uses gnome-keyring's env)
- power:
  Use libcanberra as well (see bug 780604)
- print-notifications
  Doesn't use GTK+ in the daemon, but in a sub-daemon (see bug 772767), even then it doesn't require a gtk_init(), just linking
- wacom
  Doesn't use GTK+ any more
- xrandr
  Uses GDK, rather than GTK+
- xsettings
  Uses GDK rather than GTK+

In short, we need to fix a bunch of plugins to not use libcanberra-gtk by default, then we can remove their uses of GTK+ altogether. None of the plugins use GTK+ to display UI.

The only blocker is the media-keys usage. I'm fairly certain non-D-Bus activated/non-sandboxed apps will inherit the empty theme.
Comment 15 Christian Hergert 2017-03-27 21:24:37 UTC
Created attachment 348837 [details] [review]
common: avoid loading Adwaita CSS theme into memory

The various Gtk programs are not dependent on any specific theme being
loaded. Therefore, the parsing the Adwaita CSS theme (which is quite a
detailed theme) is unnecessary and a few MB of overhead to each gsd
subprocess.

By setting the GTK_THEME environment variable in main() and providing an
alternate CSS file (which is empty), we can force Gtk to never load the
default theme, but instead our empty theme. This is important as otherwise
GtkSettings can force-load Adwaita upon first use, and that fragments the
heap.
Comment 16 Bastien Nocera 2017-03-28 09:53:15 UTC
Review of attachment 348837 [details] [review]:

Otherwise looks good.

::: plugins/common/daemon-skeleton-gtk.h
@@ +196,3 @@
+         * https://bugzilla.gnome.org/show_bug.cgi?id=780555
+         */
+        disable_default_gtk_theme ();

Seriously fugly, but that might have to do. Can you make the variable static to a single function, and pass an enable/disable boolean to it?

We can ignore the leak on exit.
Comment 17 Bastien Nocera 2017-03-28 09:55:50 UTC
Created attachment 348873 [details] [review]
housekeeping: Don't init GTK+

We don't use it.
Comment 18 Bastien Nocera 2017-03-28 09:55:55 UTC
Created attachment 348874 [details] [review]
print-notifications: Don't init GTK+

We don't use it.
Comment 19 Bastien Nocera 2017-03-28 09:56:01 UTC
Created attachment 348875 [details] [review]
wacom: Don't init GTK+

We don't use it.
Comment 20 Bastien Nocera 2017-03-28 10:02:03 UTC
Created attachment 348876 [details] [review]
common: avoid loading Adwaita CSS theme into memory

The various Gtk programs are not dependent on any specific theme being
loaded. Therefore, the parsing the Adwaita CSS theme (which is quite a
detailed theme) is unnecessary and a few MB of overhead to each gsd
subprocess.

By setting the GTK_THEME environment variable in main() and providing an
alternate CSS file (which is empty), we can force Gtk to never load the
default theme, but instead our empty theme. This is important as otherwise
GtkSettings can force-load Adwaita upon first use, and that fragments the
heap.
Comment 21 Bastien Nocera 2017-03-28 10:03:10 UTC
Attachment 348873 [details] pushed as 0db2541 - housekeeping: Don't init GTK+
Attachment 348874 [details] pushed as 5125fd0 - print-notifications: Don't init GTK+
Attachment 348875 [details] pushed as 7fb9a40 - wacom: Don't init GTK+
Attachment 348876 [details] pushed as 2088d63 - common: avoid loading Adwaita CSS theme into memory
Comment 22 Jeremy Bicha 2017-04-03 02:20:44 UTC
Could these commits be pushed to the gnome-3-24 branch too?
Comment 23 Bastien Nocera 2017-04-03 07:42:29 UTC
(In reply to Jeremy Bicha from comment #22)
> Could these commits be pushed to the gnome-3-24 branch too?

No, they need far more testing than what's been done.