1470983 - Process LookAndFeel data in the parent process

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Kris Maglione [:kmag]]

On Linux, loading the GTK style system to setup LookAndFeel currently allocates over 900K of unreported per process. I'm not sure of the numbers for other platforms. I suspect it's less on most of them, given GTK CSS style system, but I suspect it's still non-trivial.

It would be nice to load this data in the parent and share the values we need with the content process. Even better if we can do that as shared memory snapshots.

This should also, in theory, help content startup perf, since creating a bunch of widgets to calculate their look-and-feel properties is pretty expensive.

Comment 1

[Eric Rahm [:erahm]]

This is one of the larger sources of per content process overhead, Jim do you know who can look at this? It seems like we might want to redirect to the layout team.

Comment 2

[Jim Mathies [:jimm]]

(In reply to Eric Rahm [:erahm] (please no mozreview requests) from comment #1)
> This is one of the larger sources of per content process overhead, Jim do
> you know who can look at this? It seems like we might want to redirect to
> the layout team.

Hmm, I don't have anyone with the cycles for this currently. We could ask the redhat engineers if they can help out. Besides that, I'm not sure who to turn to. We really don;t have any owners for gtk right now.

Comment 3

[Karl Tomlinson (:karlt)]

Note that we'd need to do something for drawing also, or GTK will still load this data.

This seems to be another reason for bug 1411425.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

We'll want some way to ship the theme info to the child process for bug 1129492. It's possible to run the content processes in headless mode and never initialize GTK (bug 1640345), but that uses HeadlessLookAndFeel which doesn't match the browser chrome. (Text selection color is the main difference I've noticed, but there are probably others.)

Comment 5

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I have a prototype of this that seems to at least partially work.

Comment 6

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Bug 1470983 - Prelude: Remove use of gtk_widget_get_settings in LookAndFeel.

The GTK nsLookAndFeel implementation uses gtk_widget_get_settings
on temporary widgets that aren't attached to anything, which the
documentation says not to do. Empirically, this seems to return the
same settings object as gtk_settings_get_default.

Explicitly using gtk_settings_get_default is useful for remote
look-and-feel, so that the parent process can register for property
changes on that object to know when to update the child processes.

Comment 7

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Bug 1470983 - WIP for fully remote LookAndFeel. DO NOT LAND.

TODO:

• Comments!
  ** There are subtle ordering things in content process startup.
  ** Also the "compressed" representation is a little odd.
• Testing??
  ** Non-default settings? Which ones?
  ** Dynamic changes (how does that work?)
• Pref off on non-(GTK and Nightly) or something

Comment 8

[Cameron McCormack (:heycam)]

(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #7)

** Also the "compressed" representation is a little odd.

I was curious how many L&F values we tend to have present. Testing showed the following, on a new profile:

Windows: Int 49/65, Color  84/121
macOS:   Int 42/65, Color 101/121
Linux:   Int 47/65, Color  88/121

Windows is using the least -- that's 53 values of 32 bits each = 212 bytes we're saving in values by using the representation in the patch. That doesn't seem that significant, so it seems like the simplicity of having all int and color values taking space, and a BitSet indicating whether one is present, might be worth the extra space.

I didn't test floats (there are only three) or fonts (which I think should be present on all platforms).

Comment 9

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I'm also collapsing identical values. I think my main concern there was the font info, where we have 18 selectors but the GTK backend maps that down to at most 4 distinct values, and the font info contains an nsString.

Empirically, on Linux:

• 14/65 distinct ints
• 23/120 distinct colors (more than I expected)
• 2/3 distinct floats
• 1/17 distinct fonts (name length: 6)

Given that sizeof(LookAndFeelFont) == 40, the 8-byte string header + 6 UTF-16 code units rounds up to a 32-byte allocation, each compressed element still takes 1 byte, and the 4 extra nsTArray overheads (just counting the pointer and header, not trying to reason about malloc overhead here):

(rr) p (65-14)*3 + (120-23)*3 + 1*3 + 16*(39+32) - 4*16
$23 = 1519

Windows is similar:

• 17/65 distinct ints
• 21/120 distinct colors
• 1/3 distinct floats
• 2/17 distinct fonts (name lengths: 8 (x11), 14 (x4))

So, ~1.5kB per process, maybe a little more if the fonts have longer names. Not nothing in the post-Fission world, but maybe not worth the complexity / code size increase after all.

Comment 10

[Cameron McCormack (:heycam)]

OK, in my initial reading I overlooked the fact that it's storing only unique values. I'm going to leave it in.

Comment 11

[Cameron McCormack (:heycam)]

Taking Jed's patch to polish up.

Comment 12

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ from comment #7)

** There are subtle ordering things in content process startup.

What this was referring to is how ContentChild::RecvSetXPCOMProcessAttributes happens before the LookAndFeel is instantiated (or at least I hope it's a happens-before relationship). The existing “cache” path stores the data into a global variable, and then the LookAndFeel picks it up when it's constructed. The path I added is a little different: constructs the RemoteLookAndFeel object at RecvSetXPCOM… time, stores that in a different global, and then later when the LookAndFeel would normally be constructed, the existing RemoteLookAndFeel is used instead. In hindsight that might be unnecessary complication; I wasn't familiar with this area of the code when I started working on it.

Comment 13

[Cameron McCormack (:heycam)]

Thanks for clarifying that comment. I'll try to make the RemoteLookAndFeel instantiation more similar to the LNF cache path.

Comment 14

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/jobs?repo=try&revision=5221c1ca62bd0a18364fef0d5fdee3b9f7021f46

Comment 15

[Cameron McCormack (:heycam)]

I'm not really sure what to do with automated testing, as we don't have a way to change Gtk settings to test our response to them. (Well, I guess we could add some test-only C++ functions that call g_object_set(GtkSettings*, ...) to do this.) I've tested manually for both Gtk theme changes and other settings (like cursor blink speed), and they work.

Comment 16

[Cameron McCormack (:heycam)]

Attachment: Bug 1470983 - Remote all LookAndFeel values for the Gtk backend.

This adds a new LookAndFeel implementation, RemoteLookAndFeel, which can
be used in content processes and is supplied with all of its values by the
parent process.

Co-authored-by: Cameron McCormack <[e-mail]>

Comment 17

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/jobs?repo=try&revision=249ce42c6bf429d7c553da8c7f1c17b9c5fc7a29

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1fcf791ba732
Prelude: Remove use of gtk_widget_get_settings in LookAndFeel. r=heycam
https://hg.mozilla.org/integration/autoland/rev/2193d5d258b4
Remote all LookAndFeel values for the Gtk backend. r=spohl,jld

Comment 19

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/1fcf791ba732
https://hg.mozilla.org/mozilla-central/rev/2193d5d258b4

Comment 20

[Cameron McCormack (:heycam)]

(In reply to Kris Maglione [:kmag] from comment #0)

On Linux, loading the GTK style system to setup LookAndFeel currently
allocates over 900K of unreported per process. I'm not sure of the numbers
for other platforms. I suspect it's less on most of them, given GTK CSS
style system, but I suspect it's still non-trivial.

It would be nice to load this data in the parent and share the values we
need with the content process. Even better if we can do that as shared
memory snapshots.

We didn't in the end use a shared memory snapshot, so there is still some per-content-process memory usage for the LookAndFeel tables. Per comment 9 it's around 1.5 KiB per process.

https://bugzilla.mozilla.org/show_bug.cgi?id=1680175#c4 reports that we saved around 400 KiB per content process with this bug's patches. I guess it'll be more once we don't initialize Gtk at all and switch to using the non-native theme.

This should also, in theory, help content startup perf, since creating a
bunch of widgets to calculate their look-and-feel properties is pretty
expensive.

Looks like it reduced content process startup time by 12-14% / 16 ms on the CI machines.