+++ This bug was initially created as a clone of Bug #1199215 +++

Comment on attachment 8662647 [details] [diff] [review] Implement JS::ubi::Node::size for JSScript referents Review of attachment 8662647 [details] [diff] [review]: ----------------------------------------------------------------- I think it's totally your judgement call what to include in these sizes, but the things I noticed missing when comparing to JSScript::finalize are - script counts map entry - debug script - lazy script cache entry If you want to ignore those and land as-is, I'm totally cool with that. (In my quick skimming, I couldn't see a place where about:memory scans those maps separately, but I didn't look that hard.) ::: js/src/builtin/TestingFunctions.cpp @@ +2477,5 @@ > + > + { > + // We can't tolerate the GC moving things around while we're using a > + // ubi::Node. Check that nothing we do causes a GC. > + JS::AutoCheckCannotGC autoCannotGC; Should we declare JS::ubi::Node to be a GC pointer to the hazard analysis? Then you wouldn't need to do anything here, but I don't know if there are situations where you *do* hold onto a node across a GC. ::: js/src/jit-test/tests/heap-analysis/byteSize-of-scripts.js @@ +42,5 @@ > +} > + > +print("byteSizeOfScript(f2) = " + byteSizeOfScript(f2)); > +assertEq(byteSizeOfScript(f2) > 1, true); > +assertEq(byteSizeOfScript(f2) > byteSizeOfScript(f1), true); Heh. Yeah, anything more than this would probably be more annoying than helpful.

(In reply to Steve Fink [:sfink, :s:] from comment #2) > ::: js/src/builtin/TestingFunctions.cpp > @@ +2477,5 @@ > > + > > + { > > + // We can't tolerate the GC moving things around while we're using a > > + // ubi::Node. Check that nothing we do causes a GC. > > + JS::AutoCheckCannotGC autoCannotGC; > > Should we declare JS::ubi::Node to be a GC pointer to the hazard analysis? > Then you wouldn't need to do anything here, but I don't know if there are > situations where you *do* hold onto a node across a GC. JS::ubi::Nodes backed by the live heap should be considered gc pointers, but JS::ubi::Nodes backed by offline heap snapshots should not. So far, we have been writing our analyses to work with both kinds, so perhaps it would make sense? Most everything goes through JS::ubi::BreadthFirst, which requires a reference to an AutoCheckCannotGC, so I think we're pretty safe, but yeah it might make sense to make JS::ubi::Node a gc pointer itself.