Closed
Bug 712835
Opened 12 years ago
Closed 12 years ago
Add a memory reporter for the nsAtomTable
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: n.nethercote, Assigned: n.nethercote)
Details
(Whiteboard: [MemShrink])
Attachments
(3 files)
1.25 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
5.32 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
5.31 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
DMD says it's reasonable prominent: Unreported: 1 block(s) in record 9 of 17967 262,144 bytes (262,144 requested / 0 slop) 0.25% of the heap (7.19% cumulative unreported) at 0x402A063: malloc (vg_replace_malloc.c:263) by 0x403C0A4: moz_malloc (mozalloc.cpp:113) by 0x7A4126C: PL_DHashAllocTable (pldhash.cpp:114) by 0x7A41AAB: ChangeTable(PLDHashTable*, int) (pldhash.cpp:564) by 0x7A41D26: PL_DHashTableOperate (pldhash.cpp:649) by 0x7A565D9: GetAtomHashEntry(unsigned short const*, unsigned int) (nsAtomT able.cpp:403) by 0x7A56A0F: NS_NewAtom(nsAString_internal const&) (nsAtomTable.cpp:534) by 0x68B4D95: do_GetAtom(nsAString_internal const&) (nsIAtom.h:199) That's just the hash table storage, which I'm sure can get substantially bigger with more browsing. There's also the AtomImpls pointed to by the hash table entries, and then the strings hanging off the AtomImpls, but I'll need to look more closely to see if they actually belong to the AtomImpls.
Assignee | ||
Comment 1•12 years ago
|
||
This patch just adds nsStringBuffer::SizeOfIncludingThisIfUnshared(), which the next patch will use.
Attachment #583974 -
Flags: review?(khuey)
Assignee | ||
Comment 2•12 years ago
|
||
This adds the "explicit/atom-table" memory reporter. With Gmail and TechCrunch open it's just over 1MB worth. I created EnsureTableExists() for an earlier version where I registered the memory reporter in nsAtomTable.cpp. That ended up having to change, but I left EnsureTableExists() in because I think it's an improvement.
Attachment #583975 -
Flags: review?(khuey)
Assignee | ||
Comment 3•12 years ago
|
||
This patch moves AtomImpl and PermanentAtomImpl out of the header, because they don't need to be there.
Attachment #583977 -
Flags: review?(khuey)
Attachment #583974 -
Flags: review?(khuey) → review+
Attachment #583975 -
Flags: review?(khuey) → review+
Comment on attachment 583977 [details] [diff] [review] patch 3 Review of attachment 583977 [details] [diff] [review]: ----------------------------------------------------------------- Why do we want this?
Comment on attachment 583977 [details] [diff] [review] patch 3 Review of attachment 583977 [details] [diff] [review]: ----------------------------------------------------------------- Ok, I guess hiding these is fine.
Attachment #583977 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 6•12 years ago
|
||
> Why do we want this?
Basic information hiding, that's all. I just figured I'd fix it while I was in the area.
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/365161a949b5 https://hg.mozilla.org/integration/mozilla-inbound/rev/4b655c46d071 https://hg.mozilla.org/integration/mozilla-inbound/rev/fa8de405cc87
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/365161a949b5 https://hg.mozilla.org/mozilla-central/rev/4b655c46d071 https://hg.mozilla.org/mozilla-central/rev/fa8de405cc87
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in
before you can comment on or make changes to this bug.
Description
•