Closed Bug 566447 Opened 10 years ago Closed 9 years ago

add presshell memory reporter

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: vlad, Assigned: vlad)

References

Details

Attachments

(1 file, 1 obsolete file)

Here's a patch that attempts to report memory in use by presshell arenas etc.  It's likely missing memory, but it's probably a good start.
Attachment #445809 - Flags: review?(bzbarsky)
So this looks ok for the most part (though I'd probably prefer to put the Size() on the arena state, since that's where the existing ifdefs live.  But having this list of presshells worries me.  It can easily make presshell teardown pretty darned slow due to having to find the entry in the list....  I guess maybe we're already there with viewmanagers, though?

This doesn't account for the memory used by the freelist hashtable in the presarena, right?
So I had the relationship backwards here -- should I instead be keeping a list of PresContexts?  Or, better yet, is there an existing way to get such a list?
Preshell and prescontext are in a 1-1 relationship except during teardown and setup.  So it doesn't much matter which one we keep track of; we can get the other from it.

I don't think there's an existing list.  My point was that a hashset might be better than a list in terms of good behavior for large numbers of objects.
Yep, sorry, there were multiple questions going in my head -- I wanted to start reporting on Bidi memory usage, which I neede dto grab from the prescontext.. wasn't sure which one was better to keep track of.  How we keep track of it doesn't matter to me -- i'll look into switching it to a hash.
Ok, here's an update -- arenas + bidi.  Uses a hashtable, and generally seems to work well.  Those who know where other memory might be hiding should be able to quickly add a few more things here, too :)
Assignee: nobody → vladimir
Attachment #445809 - Attachment is obsolete: true
Attachment #447855 - Flags: review?(bzbarsky)
Attachment #445809 - Flags: review?(bzbarsky)
Comment on attachment 447855 [details] [diff] [review]
updated presshell/prescontext memory reporter

You should probably heap-allocate the hashtable so it won't confuse our leak logging, and use sizeof(PRUnichar) instead of magic 2.  r=me with those two changes.
Attachment #447855 - Flags: review?(bzbarsky) → review+
Hmm, what do you mean by heap-allocate -- just as a bare pointer, or in a nsAutoPtr?
Doesn't matter as long as you delete it under nsLayoutStatics shutdown.
http://hg.mozilla.org/mozilla-central/rev/d1fd855f7ebe with suggested fixes.  Thanks!
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.