As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image David Baron :dbaron: ⌚️UTC-8 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 User image 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 User image 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 User image 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 User image 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.