Process LookAndFeel data in the parent process
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox86 | --- | fixed |
People
(Reporter: kmag, Assigned: heycam)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: regression, Whiteboard: [overhead:>900k])
Attachments
(2 files, 1 obsolete file)
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Updated•6 years ago
|
Updated•6 years ago
|
Comment 2•6 years ago
|
||
Comment 3•6 years ago
|
||
Comment 4•4 years ago
|
||
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•4 years ago
|
||
I have a prototype of this that seems to at least partially work.
Updated•4 years ago
|
Comment 6•4 years ago
|
||
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•4 years ago
|
||
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
Assignee | ||
Comment 8•4 years ago
|
||
(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•4 years ago
|
||
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.
Assignee | ||
Comment 10•4 years ago
|
||
OK, in my initial reading I overlooked the fact that it's storing only unique values. I'm going to leave it in.
Comment 12•4 years ago
|
||
(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.
Assignee | ||
Comment 13•4 years ago
|
||
Thanks for clarifying that comment. I'll try to make the RemoteLookAndFeel
instantiation more similar to the LNF cache path.
Assignee | ||
Comment 14•4 years ago
|
||
Assignee | ||
Comment 15•4 years ago
|
||
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.
Assignee | ||
Comment 16•4 years ago
|
||
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 <cam@mcc.id.au>
Updated•4 years ago
|
Assignee | ||
Comment 17•4 years ago
|
||
Comment 18•4 years ago
|
||
Comment 19•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1fcf791ba732
https://hg.mozilla.org/mozilla-central/rev/2193d5d258b4
Updated•4 years ago
|
Assignee | ||
Comment 20•4 years ago
|
||
(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.
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Description
•