In order to run a census analysis on anything other than the live heap graph (the notable example being offline heap snapshots) then the census analysis cannot unwrap |ubi::Node|s into their live heap thing referents.

In order to run a census analysis on anything other than the live heap graph (the notable example being offline heap snapshots) then the census analysis cannot unwrap |ubi::Node|s into their live heap thing referents.

Comment on attachment 8647690 [details] [diff] [review] Use only JS::ubi::* interfaces in census analyses Review of attachment 8647690 [details] [diff] [review]: ----------------------------------------------------------------- The description of this bug sounded much scarier than what it actually is. I was afraid you were going to be creating more cross-compartment pointers! ::: js/src/vm/DebuggerMemory.cpp @@ +1211,4 @@ > } > > void traceCount(CountBase& countBase, JSTracer* trc) override { > Count& count= static_cast<Count&>(countBase); pre-existing: spaces on both sides of '=' @@ +1242,5 @@ > CountBasePtr stackCount(entryType->makeCount()); > if (!stackCount || !count.table.add(p, allocationStack, Move(stackCount))) > return false; > } > + MOZ_ASSERT(p); Why does add() use an outparam, again? Why can't it just return the added Ptr? (never mind me, just rambling to myself)

(In reply to Steve Fink [:sfink, :s:] from comment #2) > > void traceCount(CountBase& countBase, JSTracer* trc) override { > > Count& count= static_cast<Count&>(countBase); > > pre-existing: spaces on both sides of '=' The code containing "count=" moves to a new file in bug 1194422, so I will handle this there rather than doing some convoluted rebasing just to preserve history WRT review comments. > @@ +1242,5 @@ > > CountBasePtr stackCount(entryType->makeCount()); > > if (!stackCount || !count.table.add(p, allocationStack, Move(stackCount))) > > return false; > > } > > + MOZ_ASSERT(p); > > Why does add() use an outparam, again? Why can't it just return the added > Ptr? (never mind me, just rambling to myself) Yeah...

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a05ae0d7727f

In order to run a census analysis on anything other than the live heap graph (the notable example being offline heap snapshots) then the census analysis cannot unwrap |ubi::Node|s into their live heap thing referents.

Carrying over r+ because sfink and I debugged the issue together (thanks again!). Ended up being an accidental copy that led to the ByAllocationSite's table's entries not getting updated during moving GCs properly. New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b84f9f88d93

In order to run a census analysis on anything other than the live heap graph (the notable example being offline heap snapshots) then the census analysis cannot unwrap |ubi::Node|s into their live heap thing referents.

Gah, missing namespace prefix. Rebasing is hard. https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc771fa73a26

In order to run a census analysis on anything other than the live heap graph (the notable example being offline heap snapshots) then the census analysis cannot unwrap |ubi::Node|s into their live heap thing referents.

Sigh... missed a needed namespace prefix. This should be the last one. https://treeherder.mozilla.org/#/jobs?repo=try&revision=fcba1b2b6cdd

This has caused arm bustage like ../../dist/include/js/UbiNodeCensus.h:174:9: error: 'AutoLockForExclusiveAccess' was not declared in this scope ../../dist/include/js/UbiNodeCensus.h:174:36: error: expected ';' before 'lock' ../../dist/include/js/UbiNodeCensus.h:186:56: error: 'AllocFunction' has not been declared I'm going to back this out. https://treeherder.mozilla.org/logviewer.html#?job_id=13037293&repo=mozilla-inbound

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

I think it was bug 1194422's fault. New try push with the new version of that patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=66939d410cc6