Open Bug 704372 Opened 9 years ago Updated 7 years ago
Shrink) Add about:memory reporters for per compartment shape tables
Several bits of data that used to be stored in objects have moved to per-compartment hashtables, and the memory reporters need to be updated to count them. The attached patch does this, and also fixes a problem where object dynamic slots were not measuring slop (JM only, due to a merge botch I guess) and where asking for the allocated size of an uninitialized hashtable gave a garbage value.
Attachment #576036 - Flags: review?(nnethercote)
The above numbers are as close to a direct comparison as I could get: the JM revision is immediately after merging the trunk revision (ac667309bea6) plus the attached patch. Things line up pretty well and are about what I was expecting. - Overall, the browser's 'explicit' memory usage seems to have been reduced 4-5%, with js memory going down 8-9%. - GC timing doesn't seem to have been affected much one way or the other. - Occasional purging of property tree shape tables (and to a lesser extent, dictionary shape tables) will have a bigger effect with the ObjShrink stuff than on trunk. The amount of memory used by the tables has increased (due to more linear searches, I guess, triggering more table creation), and the size of the shapes themselves has done down a lot. Property tree tables are 31% of all shape-related memory, and dictionary tables are an additional 5%.
Assignee: general → bhackett1024
Comment on attachment 576036 [details] [diff] [review] patch Review of attachment 576036 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCJSRuntime.cpp @@ +1808,5 @@ > > ReportMemoryBytes0(MakeMemoryReporterPath(pathPrefix, stats.name, > + "shapes-extra/compartment-tables"), > + nsIMemoryReporter::KIND_HEAP, stats.shapesCompartmentTables, > + "Memory used by compartment wide tables storing shape information " I'd write "compartment-wide".
Attachment #576036 - Flags: review?(nnethercote) → review+
This change seems to make me crash ([@ je_malloc_usable_size ]) when I run sunspider on jaegermonkey nightly/hourlies. First bad changeset http://hg.mozilla.org/projects/jaegermonkey/rev/976c33eee3ac Windows Vista, desktop. Crashes with dirty profile in safe mode, does not crash with clean profile. My crash reports don't look like much help. The entirety of the stack: 0 mozutils.dll je_malloc_usable_size memory/jemalloc/jemalloc.c:6558 Typical crash report: https://crash-stats.mozilla.com/report/index/392c3cb1-c08e-438e-958d-b6d592111123 Some Otherguy's report, which seems to be the same issue but has more information: https://crash-stats.mozilla.com/report/index/b91f65b3-c542-4aad-9eea-0fe3b2111122 HTH.
It's crashing on the je_malloc_usable_size() call in JSObject::dynamicSlotSize(). My guess is that je_malloc_usable_size() is being passed a pointer that not's a valid heap block pointer. bhackett, could |slots| ever not point to a heap block in this case?
Oops, the call is passing an internal pointer rather than the head of the malloc'ed block itself. Fix: https://hg.mozilla.org/projects/jaegermonkey/rev/28bd51ce18cb
(In reply to Brian Hackett (:bhackett) from comment #8) > Fix: > > https://hg.mozilla.org/projects/jaegermonkey/rev/28bd51ce18cb WFM
You need to log in before you can comment on or make changes to this bug.