691189 - Investigate reducing calls to malloc() from nsCSSSelectorList::AddSelector

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Justin Lebar (not reading bugmail)]

According to my preliminary measurements from bug 688979, nsCSSSelectorList::AddSelector can be responsible for up to 2% of all mallocs in the browser.  This makes it one of the five or so most malloc-y callsites (falling after nsTArray, strings, hunspell, and glib).

I'm not familiar with this code, but it looks like it stores nsCSSRules as a linked list.  I wonder if we could use an nsAutoTArray here instead.

Comment 1

[Justin Lebar (not reading bugmail)]

41718 malloc's ( 2%):
  malloc at src/memory/jemalloc/jemalloc.c:6075
  moz_xmalloc at src/memory/mozalloc/mozalloc.cpp:104
  nsCSSSelectorList::AddSelector(unsigned short) at src/layout/style/StyleRule.cpp:833

33171 malloc's ( 1%):
  malloc at src/memory/jemalloc/jemalloc.c:6075
  moz_xmalloc at src/memory/mozalloc/mozalloc.cpp:104
  nsCSSSelector::AddClass(nsString const&) at src/layout/style/StyleRule.cpp:407

To put this in perspective, these two sites together are about 75,000 malloc's, while nsCStrings are responsible for about 98,000 malloc's.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I could have sworn we had existing mail threads and maybe even bugs on this, but I can't locate them...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Ah, there was data in bug 551477 and I had a note-to-self mail.  I know dbaron has some concrete thoughts here, though.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Not all that concrete, but more along the lines that we should consider draining the swamp instead of swatting the mosquitoes with flyswatters.

Comment 5

[Justin Lebar (not reading bugmail)]

FWIW, the two callsites in comment 1 are the only sites in StyleRule.cpp which show up in the top 100 malloc sites.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Most of the selector allocation probably comes from sites in the CSS parser; it's only this piece where the parser happens to do the allocation by calling a method that's implemented in a different file.

And, to be clear, in comment 4, what I meant is that we should consider, without reference to the current code, how one ought to implement a space-efficient representation of CSS selectors such that the things we need to do with them are still fast (and hopefully faster).  I tend to think a member per thing-we-might-have is the wrong approach, and we could have a fused-allocation structure with parts of the fused allocation indicating what's coming next, and perhaps a few things that make it easy to skip around (in particular, the size of each compound selector (in selectors4 terminology) / sequence of simple selectors (in css3-selectors terminology) / simple selector (in CSS1/CSS2.1 terminology)).