Open Bug 704372 Opened 9 years ago Updated 7 years ago

(ObjShrink) Add about:memory reporters for per compartment shape tables


(Core :: JavaScript Engine, defect)

Not set




(Reporter: bhackett1024, Assigned: bhackett1024)



(3 files)

Attached patch patchSplinter Review
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%.
Comment on attachment 576036 [details] [diff] [review]

Review of attachment 576036 [details] [diff] [review]:

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1808,5 @@
>      ReportMemoryBytes0(MakeMemoryReporterPath(pathPrefix,,
> +                                              "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

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:

Some Otherguy's report, which seems to be the same issue but has more information:

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:
(In reply to Brian Hackett (:bhackett) from comment #8)
>  Fix:

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