Lazily initialize the rulehash tables

RESOLVED FIXED in mozilla11

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

Trunk
mozilla11
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment, 1 obsolete attachment)

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.
Blocks: 681201
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...
Created attachment 573072 [details] [diff] [review]
Initialize the rulehash's hashtables lazily.
Attachment #573072 - Flags: review?(dbaron)
Priority: -- → P2
Once bug 698968 lands, I'll need to merge this patch to it a bit in the memory reporter parts.
Depends on: 698968
Whiteboard: [need review] → [need review][MemShrink]
Depends on: 689443
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?
> It looks like we need to propagate the malloc function thing all the way through the rule cascade 
> sizeof stuff, right?

Yes.
(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.
> It might be easier for you wait for me to do that?

Sounds good to me. Please do cc me on that!
Depends on: 705987
Whiteboard: [need review][MemShrink] → [need review][MemShrink:P2]
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.
Attachment #573072 - Flags: review?(dbaron) → review+
> looks like you have some merging to do relative to the SizeOf code, though

Yeah, see comment 4 through comment 7.
Whiteboard: [need review][MemShrink:P2] → [need dependencies fixed][MemShrink:P2]
Created attachment 580315 [details] [diff] [review]
Initialize the rulehash's hashtables lazily.

Merged on top of bug 705987.
Attachment #573072 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/28fb37461530
Flags: in-testsuite-
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [need dependencies fixed][MemShrink:P2] → [MemShrink:P2]
Target Milestone: --- → mozilla11
Version: 9 Branch → Trunk
https://hg.mozilla.org/mozilla-central/rev/28fb37461530
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.