Open
Bug 1441292
Opened 7 years ago
Updated 9 months ago
Consider making more atoms static
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
NEW
People
(Reporter: bholley, Unassigned)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [Memshrink:P2][overhead:noted])
Right now, static atoms are the things that we need to use from C++ somewhere. However, when a content process loads a mostly-empty page, there are several hundred kilobytes of dynamic atoms that end up in the table.
We should measure whether there are certain dynamic atoms that often/always end up in the table. If so, we should consider making those static atoms, since then they'll live in the binary and be shared across all processes.
Reporter | ||
Updated•7 years ago
|
Whiteboard: [Memshrink]
Reporter | ||
Comment 1•7 years ago
|
||
Specifically, stubbing out the overhead of the atom tables themselves in the memory reporter code, I measure 146k of dynamic atoms on [1]. We should figure out what those are.
[1] http://bholley.net/testcases/fileuri.html
![]() |
||
Comment 2•7 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #0)
> Right now, static atoms are the things that we need to use from C++
> somewhere. However, when a content process loads a mostly-empty page, there
> are several hundred kilobytes of dynamic atoms that end up in the table.
>
> We should measure whether there are certain dynamic atoms that often/always
> end up in the table. If so, we should consider making those static atoms,
> since then they'll live in the binary and be shared across all processes.
Except: static atoms are "static" in the sense that they are known ahead of time, but they are currently not "static" in the sense of "statically allocated". Bug 1411469 is open for fixing that. Once that bug is done, then it makes sense to convert some dynamic atoms to static atoms.
Depends on: 1411469
![]() |
||
Comment 3•7 years ago
|
||
So as a quick test, I just logged what dynamic atoms are created starting the browser and loading the page from comment 1. Data is at <https://pastebin.mozilla.org/9078664>. Quick breakdown:
Total dynamic atoms created: 2886
Things of the form "moz-nullprincipal:{UUID}": 155
"ws" and "wss": 6 (3 times each, so not hanging out for long).
Counter styles from layout/style/res/counterstyles.css: ~70
Various url-like things (principal origins?): At least 38
Various command names (Browser:*, cmd_*): ~55
Various context menu and keyboard shortcut stuff
Various CSS classes...
I'm going to try doing this in the content process only.
![]() |
||
Comment 4•7 years ago
|
||
Content process only has 232 dynamic atoms: https://pastebin.mozilla.org/9078668
"moz-nullprincipal:{UUID}": 30
Event names ("on*"): 60
Counter styles: ~70
Some CSS classes from UA stylesheets [1]
and a few other minor things.
[1] caption-box, moz-collapsed, text-link, radio-label, checkbox-label, toolbarbutton-multiline-text, scale-thumb, scale-slider, anonymous-div, preview-div, inherit-overflow, inherit-scroll-behavior, parsererror
![]() |
||
Comment 5•7 years ago
|
||
The nullprincipals are hard, obviously. ;)
We should definitely add static atoms for the counter styles.
Reporter | ||
Comment 6•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #5)
> The nullprincipals are hard, obviously. ;)
Yeah.
> We should definitely add static atoms for the counter styles.
Agreed.
I wonder if there's anything we can do about the event names. I'd be curious to see a dump of the SM atom table, and see if those atoms were also there. If so, we might consider merging the tables...
![]() |
||
Comment 7•7 years ago
|
||
The problem is thatif you do addEventListener("foo", something), the literal string (and atom) in SM is "foo" but the atom the event listener manager creates is "onfoo". Maybe we can change the atoms we use there, of course....
Reporter | ||
Comment 8•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #7)
> The problem is thatif you do addEventListener("foo", something), the literal
> string (and atom) in SM is "foo" but the atom the event listener manager
> creates is "onfoo". Maybe we can change the atoms we use there, of
> course....
Yeah, I was thinking something along those lines. Would be worth first measuring the overlap between the two atom tables (in both clean startup and heavily-loaded configurations) to determine whether the potential savings would warrant the complexity.
Boris, given that you already have the measurement setup here for gecko atoms, would you mind doing this at some point, and filing a bug if there's significant overlap?
Flags: needinfo?(bzbarsky)
![]() |
||
Comment 9•7 years ago
|
||
OK, so I redid some measurements now including JS atoms. I included the chrome process, because in JS it's not trivial to tell what process you're in. But I excluded duplicates inside the sets of "JS atoms", "static atoms", "dynamic atoms" (including cross-process duplicates).
I see 2722 dynamic atoms, 2706 static atoms, 55080 JS atoms when doing the test from comment 1. There is no overlap between dynamic and static atoms (good!). There are 887 atoms that are both JS and static atoms. There are 925 atoms that are both JS and dynamic atoms.
I filed bug 1441737 in case we want to think about it.
Blocks: 1441736
Flags: needinfo?(bzbarsky)
Comment hidden (obsolete) |
Updated•7 years ago
|
Whiteboard: [Memshrink] → [Memshrink:P2]
Updated•7 years ago
|
Whiteboard: [Memshrink:P2] → [Memshrink:P2][overhead:noted]
Comment 11•6 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #5)
We should definitely add static atoms for the counter styles.
Bug 1519011 did this.
Depends on: 1519011
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•