Last Comment Bug 700914 - Lazily initialize the rulehash tables
: Lazily initialize the rulehash tables
Status: RESOLVED FIXED
[MemShrink:P2]
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P2 normal with 1 vote (vote)
: mozilla11
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
: Jet Villegas (:jet)
Mentors:
Depends on: 689443 698968 705987
Blocks: 681201
  Show dependency treegraph
 
Reported: 2011-11-08 19:48 PST by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2011-12-10 20:38 PST (History)
9 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Initialize the rulehash's hashtables lazily. (11.92 KB, patch)
2011-11-08 20:20 PST, Boris Zbarsky [:bz] (still a bit busy)
dbaron: review+
Details | Diff | Splinter Review
Initialize the rulehash's hashtables lazily. (10.53 KB, patch)
2011-12-08 22:14 PST, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2011-11-08 19:48:58 PST
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 [:bz] (still a bit busy) 2011-11-08 20:20:22 PST
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 [:bz] (still a bit busy) 2011-11-08 20:20:33 PST
Created attachment 573072 [details] [diff] [review]
Initialize the rulehash's hashtables lazily.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2011-11-23 10:33:08 PST
Once bug 698968 lands, I'll need to merge this patch to it a bit in the memory reporter parts.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-11-28 12:33:04 PST
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) 2011-11-28 12:45:01 PST
> 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 [:njn] 2011-11-28 14:25:11 PST
(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 [:bz] (still a bit busy) 2011-11-28 14:28:21 PST
> 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-10 2011-12-06 22:32:06 PST
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 [:bz] (still a bit busy) 2011-12-06 23:27:49 PST
> 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 [:bz] (still a bit busy) 2011-12-08 22:14:26 PST
Created attachment 580315 [details] [diff] [review]
Initialize the rulehash's hashtables lazily.

Merged on top of bug 705987.
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2011-12-09 02:30:29 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/28fb37461530
Comment 12 Ed Morley [:emorley] 2011-12-10 20:38:12 PST
https://hg.mozilla.org/mozilla-central/rev/28fb37461530

Note You need to log in before you can comment on or make changes to this bug.