Open Bug 1441292 Opened 7 years ago Updated 9 months ago

Consider making more atoms static

Categories

(Core :: XPCOM, enhancement)

enhancement

Tracking

()

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.
Whiteboard: [Memshrink]
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
(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
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.
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
The nullprincipals are hard, obviously. ;) We should definitely add static atoms for the counter styles.
(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...
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....
(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)
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)
Whiteboard: [Memshrink] → [Memshrink:P2]
Whiteboard: [Memshrink:P2] → [Memshrink:P2][overhead:noted]

(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
Depends on: 1542854
Depends on: 1542885
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.