This testcase confuses me. It should allocate 2GB or so of external strings. And stepping through in a debugger, I see us doing just that: mallocing a bunch of stringbuffers, handing each one over to the JS engine as an external string. The confusing part is that when all is done about:memory does _not_ show 2GB of heap-unclassified, and if I ask the OS how much memory the process is using it's nowhere close to 2GB. This is very confusing to me.... njn, any idea what's going on here?

Off the top of my head, I don't know what's happening. But the OS should be correct. I see 400 MiB of arrays (8 bytes per elements, which seems reasonable) but no strings. You see a bunch of strings being allocated -- do you see 5e7 of them? I can take a closer look when I get back from PTO on Jan 9 if it's still a mystery then.

> You see a bunch of strings being allocated -- do you see 5e7 of them? Oh, hmm. Good point! I bet what's going on here is that once we ion-compile we end up loop-hoisting the innerHTML get. Let me check that hypothesis.

I'm seeing a nice 85% or so heap-unclassified with this testcase.

I think that rather than overloading the finalizer function we pass in for external strings, or providing a separate function (bloating JS strings for little win), the best thing is to register some sort of callback for the JS engine memory reporting bits for when external strings are encountered during memory reporting. Not sure whether we can attribute those external strings to the web page that caused them, though even just being able to say "hey, there's a pile of external strings here!" might be a useful thing to know...

Ok, so with a bit of plumbing, that testcase now gives me: 2,850.79 MB (100.0%) -- explicit ├──2,688.96 MB (94.32%) -- js-non-window │ ├──2,680.50 MB (94.03%) -- zones │ │ ├──2,672.75 MB (93.75%) -- zone(0x118ca7000) │ │ │ ├──2,670.29 MB (93.67%) -- strings │ │ │ │ ├──2,670.29 MB (93.67%) -- string(length=61, copies=9999999, "<div x="y">/n <span>Please look at about:memory</span>/n</di") which is better than nothing, but still doesn't pin the strings on a particular website, unfortunately. I think that's a general problem with the way the JS engine reports string memory, though...

Comment on attachment 8822293 [details] [diff] [review] part 1. Add a way to set an external string memory runtime callback Review of attachment 8822293 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsapi.h @@ +734,5 @@ > + * JSStringFinalizer passed to JS_NewExternalString for this string. > + */ > +typedef size_t > +(* JSExternalStringSizeofCallback)(JSString* str, > + mozilla::MallocSizeOf mallocSizeOf); It's inconsistent with the surrounding code, but whatever, we prefer it now -- use a C++11-style typedef: using JSExternalStringSizeofCallback = size_t (*)(JSString* str, mozilla::MallocSizeOf mallocSizeOf); ::: js/src/vm/Runtime.h @@ +843,5 @@ > > /* Call this to get the name of a compartment. */ > JSCompartmentNameCallback compartmentNameCallback; > > + /* Callback for doing memory reporting on external strings */ Period at end.

Comment on attachment 8822294 [details] [diff] [review] part 2. If there is an external string memory runtime callback, call it from memory reporting code Review of attachment 8822294 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/String.cpp @@ +56,5 @@ > + // JSExternalString: Ask the embedding to tell us what's going on. If it > + // doesn't want to say, don't count, the chars could be stored anywhere. > + if (isExternal()) { > + JSRuntime* rt = runtimeFromMainThread(); > + if (rt->externalStringSizeofCallback) I'd do if (auto* cb = runtimeFromMainThread()->externalStringSizeofCallback) return cb(this, mallocSizeOf); which also happens to make clear that the callback is a function pointer and not a member function that might get passed the runtime.

Addressed those comments.

Comment on attachment 8822295 [details] [diff] [review] part 3. Pass a useful external string memory reporter to SpiderMonkey from Gecko code Review of attachment 8822295 [details] [diff] [review]: ----------------------------------------------------------------- Thank you!

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/88b94a2497dc part 1. Add a way to set an external string memory runtime callback. r=waldo https://hg.mozilla.org/integration/mozilla-inbound/rev/52d6c09777ee part 2. If there is an external string memory runtime callback, call it from memory reporting code. r=waldo https://hg.mozilla.org/integration/mozilla-inbound/rev/d3a2ffd6f792 part 3. Pass a useful external string memory reporter to SpiderMonkey from Gecko code. r=froydnj

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/defd98672958 followup. Suppress the false-positive GC analysis failure due to us doing an indirect call that the static analysis doesn't understand. r=sfink

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9c2b070d0054 another followup: fix build bustage on CLOSED TREE.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/72e5976fb87a yet another followup. Silence the opinionated spidermonkey include style checker on CLOSED TREE

Backout by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9fc7373573e9 Back out bits that should not have landed for bug 1326096 on CLOSED TREE.