I'm going to start posting patches and hope to land them incrementally.

The general strategy is to add APIs to tell the GC about malloc memory associated with a cell:

  void AddCellMemory(gc::Cell* cell, size_t nbytes, MemoryUse use);
  void RemoveCellMemory(gc::Cell* cell, size_t nbytes, MemoryUse use)

Calls to these must be balanced and this is checked in debug builds.

FreeOp is updated with methods to call RemoveCellMemory and free. The long term aim is to replace all use of the current FreeOp::free_/delete_ with these methods to ensure all memory is tracked.

To start with I'm going to add tracking for memory which can contribute significantly to the total, e.g. is very frequently used or has a size which is under user control.

Depends on D30514

Depends on D30515

This adds memory tracking for string contents while leaving the current scheme in place for the time being.

Depends on D30516

https://hg.mozilla.org/mozilla-central/rev/772b3ec0102d
https://hg.mozilla.org/mozilla-central/rev/95399bf8d949
https://hg.mozilla.org/mozilla-central/rev/54227b612212
https://hg.mozilla.org/mozilla-central/rev/01140845cccc

Since we now have precise memory accounting for externally allocated memory associated with GC things we should be able to remove use of the existing malloc counter here. This should help with cases where we trigger too many GCs because we think there is more memory associated than there really is.

== Change summary for alert #20930 (as of Mon, 13 May 2019 11:46:58 GMT) ==

Improvements:

4% raptor-tp6-twitter-firefox loadtime windows7-32-shippable opt 257.33 -> 246.29

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20930

https://hg.mozilla.org/mozilla-central/rev/441a4c70ac22

This code is debug-only, but it seems a shame to waste have this key structure take up two words when it will fix into one. This also moves the MemoryUse enum definition to gc/GCEnum.h.

This adds memory tracking for object elements. I had to swap the association in JSObject::swap. I also moved NativeObject::shrinkCapacityToInitializedLength out of line so as not to have to pull Zone.h into NativeObject.h. I don't know how performance sensitive this is - if it is I could look at this again.

Depends on D32170

Depends on D32171

Depends on D32172

https://hg.mozilla.org/mozilla-central/rev/740009023b1b

https://hg.mozilla.org/mozilla-central/rev/94dadeeee608
https://hg.mozilla.org/mozilla-central/rev/92fc3159f152
https://hg.mozilla.org/mozilla-central/rev/5585ac78378a

This changes ZoneAllocPolicy to use the new precise memory tracking rather than the old malloc counter. This works because the mozilla standard containers (HashMap, Vector, etc) already call the AllocyPolicy free_ method with the correct size. This patch tracks individual instances of ZoneAllocPolicy to check that the numbers balance.

Depends on D33000

This refactors the malloc allocation tracking so that the MemoryTracker object is only present in debug builds and the count of malloc bytes is kept separately in the Zone. This makes things a little clearer I think and removes one level of inlining.

This splits out the allocation functions and allocation tracking state into a new base class, ZoneAllocator, which lives in its own header file. We can include this for the common case of allocating malloc memory for GC things without dragging in the full complexity of Zone.h.

Depends on D33179

Depends on D33313

https://hg.mozilla.org/mozilla-central/rev/b37b6c015168
https://hg.mozilla.org/mozilla-central/rev/50769742e295
https://hg.mozilla.org/mozilla-central/rev/a4daa44cdb9c

Backed out 3 changesets (bug 1395509, bug 1555935) for causing build busateges CLOSED TREE

Backout revision https://hg.mozilla.org/integration/autoland/rev/176ac3f06690a9e5071028ec601a2e743545d045

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=351d19c8bdd3008e17af87852574ec4b9485262a&selectedJob=249689606&searchStr=linux%2Cx64%2Casan%2Cbuild-linux64-asan%2Fdebug%2C%28bd%29

Log failure https://treeherder.mozilla.org/logviewer.html#?job_id=249693971&repo=autoland

Jon can you please take a look?

https://hg.mozilla.org/mozilla-central/rev/ab12b34ff823

The change to split out ZoneAllocator messed up the MemoryTracker code that prints out what failed to be removed by making it run after the Zone destructor which will already assert in the case (but without printing useful information first).

Depends on D33974

Previously I rolled the malloc byte count into a total byte count for each zone but this may adversely affect GC scheduling (e.g. by triggering more non-incremental GCs because allocation volumes appear higher with this change). So that we can land this machinery without disturbing benchmarks too much, this patch splits out the new malloc memory accounting into a separate counter and uses the maxMallocBytes setting as the threshold (default value is 128MB vs 30MB for the GC heap threshold). This should make the behaviour closer to the original behaviour for now. We can go back and adjust the parameters later to obtain the desired behaviour.

Depends on D34180

Use memory tracking APIs to track malloc memory associated with Map and Set objects.

Depends on D34181

Use memory tracking APIs to track malloc memory associated with BigInts.

Depends on D34371

Use memory tracking APIs to track malloc memory associated with Scopes.

Depends on D34373

Use memory tracking APIs to track malloc memory associated with the different ctypes objects.

I ended up creating new public APIs because ctypes currently mostly uses our public APIs, but I actaully don't know why. I don't think it can be built standalone. Maybe this should use the internal APIs instead.

Depends on D34374

Use memory tracking APIs to track malloc memory associated with weak maps.

Depends on D34375

Use memory tracking APIs to track malloc memory associated with Shapes. I had to thread FreeOp and BaseShape through in various places so there's enough context.

Depends on D34376

Use memory tracking APIs to track malloc memory associated with module IndirectBindingMap. These are attached to ModuleObjects and ModuleNamespaceObjects.

Depends on D34377

Use memory tracking APIs to track malloc memory associated with IonScripts and BaselineScripts. This does the memory accounting when they are attached/detached from the JSScript rather than on initialisation/finalization as normally. I had to record the allocation size in the objects themselves. Hopefully this isOK.

Depends on D34551

Use memory tracking APIs to track malloc memory associated with ArgumentsObjects. This one is slightly more complicated because we can allocate these in the nursery and malloc memory is not tracked for nursery objects, so we have to do this if they get tenured.

Depends on D34552

Use memory tracking APIs to track malloc memory associated with NativeIterator objects. I had to store the initial propery count as propertiesEnd can change during the lifetime of this object.

Depends on D34553

Depends on D34554

I wanted to make VectorMatchPairs use ZoneAllocPolicy but this is also used in a bunch of places where it's not attached to a GC thing, so I left this as a todo.

Depends on D34555

https://hg.mozilla.org/mozilla-central/rev/f3e11ae5fb19
https://hg.mozilla.org/mozilla-central/rev/06ef8a540487
https://hg.mozilla.org/mozilla-central/rev/1b3207f97971

Some kinds of memory use (e.g. reg exp byte code) require multiple memory associations for a single cell. This patch adds machinery to let that happen for specific uses.

Depends on D34556

I had to store the length as the first word of the bytecode. Hopefully this is not too onerous. I also shuffled RegExpShared fields around to make the class a little smaller.

Depends on D34723

Note that we only track the for typed arrays that are not backed by an ArrayBuffer as the memory is tracked there from then on.

Depends on D34726

I'm not sure what these are, but track them anyway.

Depends on D34729

This changes the format of the trace list from using -1 as a delimter to storing the list lengths up front so that we have length information.

Depends on D34730

https://hg.mozilla.org/mozilla-central/rev/059c1897eb8f
https://hg.mozilla.org/mozilla-central/rev/db2d48f8abf1
https://hg.mozilla.org/mozilla-central/rev/e4f684303335
https://hg.mozilla.org/mozilla-central/rev/0943e5feaa8d
https://hg.mozilla.org/mozilla-central/rev/1a2985fdef25
https://hg.mozilla.org/mozilla-central/rev/82c5c40a8881
https://hg.mozilla.org/mozilla-central/rev/c9c24c9e108c

Backed out 7 changesets (bug 1395509) for SM build failures on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/d8a83a5f0fb9165f97d8000976bb2241d44cda34

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed%2Cbusted%2Cexception&revision=9de6fe9f087eb386f829300206084da2a0861ea3&selectedJob=251889998

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=251889998&repo=mozilla-inbound&lineNumber=2659

Log snippet:

[task 2019-06-14T13:48:17.817Z] File "/usr/lib/python2.7/subprocess.py", line 540, in check_call
[task 2019-06-14T13:48:17.817Z] raise CalledProcessError(retcode, cmd)
[task 2019-06-14T13:48:17.817Z] subprocess.CalledProcessError: Command '['/builds/worker/workspace/obj-haz-shell/dist/bin/js', '/builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/computeGCTypes.js', 'gcTypes.tmp', 'typeInfo.tmp']' returned non-zero exit status -11
[task 2019-06-14T13:48:17.861Z] Loaded /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/t/out/suppression/defaults.py
[task 2019-06-14T13:48:17.861Z] Running gcTypes to generate ('gcTypes.txt', 'typeInfo.txt')
[task 2019-06-14T13:48:17.861Z] PATH="/builds/worker/workspace/sixgill/usr/bin:${PATH}" SOURCE='/builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/t/suppression' ANALYZED_OBJDIR='/builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/t/out/suppression' LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/builds/worker/workspace/obj-haz-shell/dist/bin" XDB='/builds/worker/workspace/sixgill/usr/bin/xdb.so' /builds/worker/workspace/obj-haz-shell/dist/bin/js /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/computeGCTypes.js gcTypes.txt typeInfo.txt
[task 2019-06-14T13:48:17.887Z] Traceback (most recent call last):
[task 2019-06-14T13:48:17.887Z] File "/builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/analyze.py", line 339, in <module>
[task 2019-06-14T13:48:17.887Z] run_job(step, data)
[task 2019-06-14T13:48:17.887Z] File "/builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/analyze.py", line 209, in run_job
[task 2019-06-14T13:48:17.887Z] subprocess.check_call(command, env=env(config))
[task 2019-06-14T13:48:17.887Z] File "/usr/lib/python2.7/subprocess.py", line 540, in check_call
[task 2019-06-14T13:48:17.887Z] raise CalledProcessError(retcode, cmd)

(In reply to Raul Gurzau (:RaulGurzau) from comment #58)
Steve do you know what happened here? It seems my patches broke the hazard analysis task somehow. I wasn't able to see anything useful in the logs. I'm currently trying to bisect this on try.

https://hg.mozilla.org/mozilla-central/rev/d3436ffbb75c
https://hg.mozilla.org/mozilla-central/rev/bc1b32cdf4ca
https://hg.mozilla.org/mozilla-central/rev/54817eb3c564
https://hg.mozilla.org/mozilla-central/rev/848bf60827e4
https://hg.mozilla.org/mozilla-central/rev/219909ea3e4f
https://hg.mozilla.org/mozilla-central/rev/19f38c9cdd67

Depends on D34554

Depends on D35345

Depends on D35346

This refactors freeing stubs and adds memory tracking. It also adds a back pointer to the JSObject to ForOfPIC::Chain which is slightly annoying, but I think there's only one per global so this shouldn't be too bad.

Depends on D35347

This adds tracking of malloc memory to WasmInstanceObject, WasmGlobalObject, WasmMemoryObject and ResolveResponseClosure (the straightforward cases).

Depends on D35484

https://hg.mozilla.org/mozilla-central/rev/d01efbed93ae
https://hg.mozilla.org/mozilla-central/rev/3c5dbd08506b
https://hg.mozilla.org/mozilla-central/rev/340f561967eb
https://hg.mozilla.org/mozilla-central/rev/fb304348f7b1
https://hg.mozilla.org/mozilla-central/rev/2e0419940dd6

https://hg.mozilla.org/mozilla-central/rev/442d11bc5876
https://hg.mozilla.org/mozilla-central/rev/b396c73d8978

Add memory tracking for tables and modules. Tables can grow and so update their memory use when that happens (if associated with a JSObject). Modules have lots of associated data so calculate and cache a size on creation. The size doesn't have to be exact so this reuses the serialization framework to get an approximate value.

The memory tracking needs the GC thing so I made WasmBreakpointSite hold a pointer to the Instance rather than just the DebugState. I added DebugState::getBreakpointSite for cases were we already expect this to exist, like JSScript::getBreakpointSite.

Depends on D35793

Depends on D35795

Depends on D35796

Depends on D34555

I realised I was using the wrong threshold to compare the malloc bytes counter against in ZoneAllocator::maybeMallocTriggerZoneGC. This was using the GC heap bytes threshold. In other places this correctly uses gcMallocThreshold.

Patch to allow ZoneAllocPolicy insstance to be moved after having memory accounted to it. This is useful in the situation where a data structure is populated before being attached to a GC thing by moving it. The code will currently assert (in unregisterPolicy()) if a ZoneAllocPolicy is copied after having memory accounted to it. This could be allowed but it seems better to force users to move in this situation.

Depends on D36175

The patch switches some ctypes data structures to ZoneAllocPolicy so that their memory use is tracked against the zone. The creation of heapHash in StructType::DefineInternal relies on the ZoneAllocPolicy being movable after having memory accounted to it.

Depends on D36177

https://hg.mozilla.org/mozilla-central/rev/d7cd95d34fb6
https://hg.mozilla.org/mozilla-central/rev/68eecc44047f

https://hg.mozilla.org/mozilla-central/rev/f7088c17e423

This replaces SystemAllocPolicy with ZoneAllocPolicy for things related to zones where possible. There are a few places where this isn't possible at the moment due to header include order. I'll try and revisit these later after removing the current malloc memory tracking as this simplifies the situation.

Depends on D36178

This restores the use of GC invocation kind that was inadvertantly removed by the patch in bug 1395509 comment 33 from the call to ZoneAllocator::updateAllGCThresholds in GCRuntime::endSweepingSweepGroup. Also refactors ZoneMallocThreshold::computeZoneTriggerBytes a little.

Depends on D36923

FF 69 is moving to beta. I'm going to close this bug and continue this work in bug 1564072. Outstanding patches will land as part of that bug.

Comment on attachment 9073822 [details]
Bug 1395509 - Track malloc memory used by WasmBreakpoingSites r=luke?

Revision D35795 was moved to bug 1564072. Setting attachment 9073822 [details] to obsolete.

Comment on attachment 9073823 [details]
Bug 1395509 - Track malloc memory used by Breakpoints r=jimb?

Revision D35796 was moved to bug 1564072. Setting attachment 9073823 [details] to obsolete.

Comment on attachment 9073825 [details]
Bug 1395509 - Deprecate FreeOp APIs that don't track memory and rename them to make their use obvious r=sfink?

Revision D35798 was moved to bug 1564072. Setting attachment 9073825 [details] to obsolete.

Comment on attachment 9075979 [details]
Bug 1395509 - Associate more per-zone memory with the zone r=sfink?

Revision D36923 was moved to bug 1564072. Setting attachment 9075979 [details] to obsolete.

Comment on attachment 9075980 [details]
Bug 1395509 - Fix use of invocation kind that was previously lost and refactor malloc threshold updates r=sfink?

Revision D36924 was moved to bug 1564078. Setting attachment 9075980 [details] to obsolete.