Last Comment Bug 676048 - Report RuleCascadeData and friends in about:memory
: Report RuleCascadeData and friends in about:memory
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Kyle Huey [:khuey] (
Depends on: 702984
Blocks: 671299 676057
  Show dependency treegraph
Reported: 2011-08-02 12:12 PDT by Kyle Huey [:khuey] (
Modified: 2011-11-16 09:42 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (6.21 KB, patch)
2011-08-02 12:12 PDT, Kyle Huey [:khuey] (
bzbarsky: review+
Details | Diff | Splinter Review
Patch w/review comments (6.72 KB, patch)
2011-08-02 13:19 PDT, Kyle Huey [:khuey] (
khuey: review+
Details | Diff | Splinter Review

Description Kyle Huey [:khuey] ( 2011-08-02 12:12:50 PDT
Created attachment 550155 [details] [diff] [review]

Add up lots of hashtables and arrays
Comment 1 Boris Zbarsky [:bz] 2011-08-02 12:35:34 PDT
Comment on attachment 550155 [details] [diff] [review]

> +  // We don't bother enumerating this one since all it does is hold a ref to
> an atom

Add a comment in the entry class to fix that if the entry class ever changes?

Please file a followup bug on me to nuke AttributeSelectorEntry and just use AtomSelectorEntry for the attributes table?  I have no idea why that wasn't done in bug 645491 or some followup...  I think it was because I had some other changes to revamp all this selector hashtable stuff but those never landed.

Please use static_cast, not C-style casts.

>+  for (RuleCascadeData * const *cascadep = &mRuleCascades, *cascade;
>+       (cascade = *cascadep); cascadep = &cascade->mNext) {

How about:

  for (RuleCascadeData* cascade = mRuleCascades; cascade; 
       cascade = cascade->mNext) {

It's not like you have to _modify_ the list, unlike RefreshRuleCascades.

r=me with those changes.
Comment 2 Kyle Huey [:khuey] ( 2011-08-02 13:19:34 PDT
Created attachment 550180 [details] [diff] [review]
Patch w/review comments

Review comments addressed.  Turns out we should traverse those other hashtables, since RuleHashTagTableEntry inherits from RuleHashTableEntry, we were traversing.
Comment 3 Kyle Huey [:khuey] ( 2011-08-02 13:22:41 PDT
Comment 4 Boris Zbarsky [:bz] 2011-08-02 13:33:57 PDT
Comment on attachment 550180 [details] [diff] [review]
Patch w/review comments

This could still use a comment on RuleHashTagTableEntry that says that if any members are added the traverse code will need changing...
Comment 5 Kyle Huey [:khuey] ( 2011-08-10 08:13:03 PDT
Added a few more comments.

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