700914 - Lazily initialize the rulehash tables

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

We create a bunch of rulehashes now, esp. in the UA level for a page.  Most of them don't use most of the hashtables, so it would make sense to initialize them more lazily.

We should probably also reduce the default size of the tag table to 16.  See bug 681201 comment 24 for data.

Comment 1

[Boris Zbarsky [:bzbarsky]]

With the upcoming patch, we should limit the damage to sizeof(RuleHash)*8 or so.  Over here, sizeof(RuleHash) == 148 (in a 64-bit build).  128 of that is the 4 PLDHashTables.

148*8 = 1184, so figure 1KB per blank tab.  I guess that's better than the old 46KB per blank tab...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Initialize the rulehash's hashtables lazily.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Once bug 698968 lands, I'll need to merge this patch to it a bit in the memory reporter parts.

Comment 4

[Boris Zbarsky [:bzbarsky]]

So I'm trying to merge this to the changes in bug 698968.  It looks like we need to propagate the malloc function thing all the way through the rule cascade sizeof stuff, right?

Comment 5

[Justin Lebar (not reading bugmail)]

> It looks like we need to propagate the malloc function thing all the way through the rule cascade 
> sizeof stuff, right?

Yes.

Comment 6

[Nicholas Nethercote [inactive]]

(In reply to Boris Zbarsky (:bz) from comment #4)
> So I'm trying to merge this to the changes in bug 698968.  It looks like we
> need to propagate the malloc function thing all the way through the rule
> cascade sizeof stuff, right?

Oh, I'm part-way through updating the existing layout reporters to use mallocSizeOf, but I haven't filed a bug yet.  It might be easier for you wait for me to do that?  I can CC you on the bug when I file it, probably later today.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> It might be easier for you wait for me to do that?

Sounds good to me. Please do cc me on that!

Comment 8

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

Comment on attachment 573072 [details] [diff] [review]
Initialize the rulehash's hashtables lazily.

r=dbaron

looks like you have some merging to do relative to the SizeOf code, though -- I think you want PL_DHashTableSizeOfExcludingThis, but you should make the const change to both.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> looks like you have some merging to do relative to the SizeOf code, though

Yeah, see comment 4 through comment 7.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Initialize the rulehash's hashtables lazily.

Merged on top of bug 705987.

Comment 11

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/28fb37461530

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/28fb37461530