Right now, we analyze scripts when they first execute, to keep track of the types of values that have been observed and update the associated type sets on the script. With bug 804676 fixed this behavior is no longer required by Ion, but is still required by JM+TI (which infallibly reuses the inferred types at loop headers during OSR). If JM is removed then this analysis could be postponed until the script is baseline compiled, which would allow plenty of time to accumulate type information before the script is Ion compiled. Postponing the generation of analysis information (and additionally, folding it into the baseline script itself) would be beneficial as a lot of code only executes once or a few times. Some sites which execute lots of loop free scripts waste a lot of memory due to this analysis information.

This sounds great and will definitely help some of the memory spikes I've seen for the non-asm.js portions of Emscripten code (boilerplate, glue, etc). Folding it into baseline compilation sounds right. Could all the analyze::Bytecodes be allocated in a LifoAllocScope released at the end of baseline compilation (thereby removing the need for the released-on-gc analysisLifoAlloc)? While this would require re-analysis on every baseline compile and Ion compile of a method:(1) IIUC baseline doesn't need to recompile much (2) for Ion, I assume the analysis time would be small compared to the cost of Ion compilation.

I'm hoping we'll be able to kill ScriptAnalysis entirely and fold the minimal amount of analysis needed into baseline compilation itself. Baseline uses the analysis info for a few things such as knowing jump targets, but I think Ion uses are even more minimal (Ion uses source notes to traverse the bytecode so figures out jump targets etc. separately) and could be easily folded into the BaselineScript.

\o/

Thinking a bit more about this, in cases where JM is disabled at compile time we could do this bug now. The script isn't analyzed until it is baseline compiled, and when monitoring bytecode results the interpreter first checks to see if there is analysis info, doing nothing if there isn't. If JM is enabled then the script still needs to be analyzed immediately for the reason outlined in comment 0. Accelerating this could be beneficial both for the browser and for b2g (if configure options are changed to disable JM completely). The latter could be especially important if, as I vaguely recall from the GC meeting today, b2g will be forking a version off trunk soon and not merge again for several months. Is anyone around with actual knowledge about this?

This patch adds a cx->jaegerCompilationAllowed() bit which works similarly to cx->typeInferenceEnabled() --- set at zone creation and not changed afterwards. If the bit is set then we do the same analysis work as usual to accumulate types for JM execution, otherwise we don't analyze until required by baseline compilation or by the 'arguments' or 'new' script properties analyses. Still a couple TODOs: - There is an exception to the above, we still analyze scripts that contain initializers to see whether they are in a loop and shouldn't have singleton type. Will fix this in a separate patch. - I still need to make sure this bit is being set properly in the browser.

Adds a try note for non-iterator loops, so that we can quickly test for whether a bytecode is inside a loop. Use this instead of analysis information for deciding to use a singleton type during object initialization.

Comment on attachment 743063 [details] [diff] [review] patch Review of attachment 743063 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, we should change the default value for the JM browser prefs though. Also please make sure you get reasonable SS numbers for --no-ion --no-baseline (to confirm JM+TI still works).

Fix browser defaults to disable JM, and remove some test detritus referencing these config options.

Comment on attachment 743092 [details] [diff] [review] part 2 Review of attachment 743092 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/BytecodeEmitter.cpp @@ +4367,5 @@ > if (EmitJump(cx, bce, op, top - bce->offset()) < 0) > return false; > > + if (!bce->tryNoteList.append(JSTRY_LOOP, bce->stackDepth, top, bce->offset())) > + return false; Bump XDR_BYTECODE_VERSION in vm/Xdr.h

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/f37eeebce2eb So far, looks like: * talos tp on every platform with talos' opaque "browser non-zero return code" a la https://tbpl.mozilla.org/php/getParsedLog.php?id=22377765&tree=Mozilla-Inbound * android jsreftest-2 crashing in regress-119909.js [@ JSC::Yarr::ByteCompiler::emitDisjunction(JSC::Yarr::PatternDisjunction*, unsigned int, unsigned int)] a la https://tbpl.mozilla.org/php/getParsedLog.php?id=22377910&tree=Mozilla-Inbound * probably Win talos xperf crashing [@ js::types::TypeSet::hasType(js::types::Type)] * perhaps talos dromaeo_css crashing [@ js::ion::BuildDominatorTree(js::ion::MIRGraph&)]

* perhaps b2g mochitest startup hangs (or crashes, or total failure to start, it can be hard to tell what it's up to)

Hmm, the hasType crash is easy to fix but it's not at all obvious what's happening with the other failures. Decoder and Gary, can you try fuzzing rev 80163a75cca5 with a variety of options that include --no-jm? I don't know if this will turn up anything besides the hasType crash but seems worth trying.

> Decoder and Gary, can you try fuzzing rev 80163a75cca5 with a variety of > options that include --no-jm? I don't know if this will turn up anything > besides the hasType crash but seems worth trying. The jsfunfuzz harness already fuzzes with various options including --no-jm.

All the ARM/b2g failures seem to stem from just turning off the methodjit pref. This is entangled with code that controls the Yarr JIT, which also gets disabled and the fallback interpreter crashes somewhere. This patch disentangles these two and on try doesn't crash in any new spots. https://tbpl.mozilla.org/?tree=Try&rev=5a16355efbde

Turn off methodjit prefs: https://hg.mozilla.org/integration/mozilla-inbound/rev/bf0a237017a9

The rest: https://hg.mozilla.org/integration/mozilla-inbound/rev/5ac1564bff87 Try run: https://tbpl.mozilla.org/?tree=Try&rev=9df6f327d8f2

(In reply to Brian Hackett (:bhackett) from comment #17) > The rest: > > https://hg.mozilla.org/integration/mozilla-inbound/rev/5ac1564bff87 > > Try run: > > https://tbpl.mozilla.org/?tree=Try&rev=9df6f327d8f2 Backed out for causing intermittent linux32 dromaeo crashes. https://hg.mozilla.org/integration/mozilla-inbound/rev/9c426bd08d28 https://tbpl.mozilla.org/php/getParsedLog.php?id=22511532&tree=Mozilla-Inbound

Different stack: https://tbpl.mozilla.org/php/getParsedLog.php?id=22511203&tree=Mozilla-Inbound

(In reply to Brian Hackett (:bhackett) from comment #17) > Try run: > > https://tbpl.mozilla.org/?tree=Try&rev=9df6f327d8f2 Retriggers on this push show the crash too.

This patch also caused regressions in V8, Kraken, and Dromaeo on multiple platforms, both times it landed. Were these expected? http://graphs.mozilla.org/graph.html#tests=[[230,131,33],[230,131,12]]&sel=1366918928204,1367523728204 https://groups.google.com/d/topic/mozilla.dev.tree-management/tgJnrydtoeA/discussion

The dromaeo failure is some existing bug in the compiler that is getting tickled by the changes to the type information here. I can't reproduce it myself, it only repros once in every 20 or so times on recent try runs, and the signature gives no obvious clues for diagnosis. I'm going to try some random things to see if they avoid the dromaeo crash, but who knows if that will work. Without some STR these patches are hosed. The attached patch gives much of the benefit, generates the same type information and is stable on try. Instead of analyzing a script to allow the interpreter to monitor bytecode results, it just computes a much smaller index -> pc map for the monitored opcodes, just enough information to efficiently allow the type sets to be updated. The amount of memory used by this map is a tiny fraction of the analysis information. The main way this is deficient vs. the earlier patch is that we still need to allocate script->types for scripts that haven't yet been compiled by baseline. script->types consumes a significant amount of memory, though nothing anywhere near as bursty as analysis memory currently is.

Comment on attachment 745584 [details] [diff] [review] allow monitoring types without analyzing scripts Review of attachment 745584 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense.

Backed out for jsreftest crashes. https://hg.mozilla.org/integration/mozilla-inbound/rev/b5e04d0c4a37 https://tbpl.mozilla.org/php/getParsedLog.php?id=22661073&tree=Mozilla-Inbound

And jit-test failures. https://tbpl.mozilla.org/php/getParsedLog.php?id=22661878&tree=Mozilla-Inbound But hey, at least this landed late in the day, so we won't be starring as many failures for the next few hours...

And Windows dromaeo. And apparently left Android completely unable to read reftest/crashtest manifests, which is a particularly curious failure.

Particularly curious, and perhaps a needs-clobber that you inherited, since later runs before you got backed out didn't have the same complete Android bustage.

This bug is cursed. https://hg.mozilla.org/integration/mozilla-inbound/rev/357af4877abd

(In reply to Matt Brubeck (:mbrubeck) from comment #22) > This patch also caused regressions in V8, Kraken, and Dromaeo on multiple > platforms, both times it landed. Were these expected? It looks like the Kraken regression was fixed in the second landing, but V8 and Dromaeo still regressed significantly. Again, was this expected? Should we back out the third landing if it causes 10-20% regressions in V8 and Dromaeo DOM?

(In reply to Matt Brubeck (:mbrubeck) from comment #31) > It looks like the Kraken regression was fixed in the second landing, but V8 > and Dromaeo still regressed significantly. Again, was this expected? On the third landing, both Kraken and V8 are fine but Dromaeo DOM and CSS both regressed around 10-15%: http://perf.snarkfest.net/compare-talos/index.html?oldRevs=081ebf4e0886,3b562fd6d653,7d3a05e286a6&newRev=357af4877abd&submit=true

(In reply to Matt Brubeck (:mbrubeck) from comment #32) > (In reply to Matt Brubeck (:mbrubeck) from comment #31) > > It looks like the Kraken regression was fixed in the second landing, but V8 > > and Dromaeo still regressed significantly. Again, was this expected? > > On the third landing, both Kraken and V8 are fine but Dromaeo DOM and CSS > both regressed around 10-15%: > http://perf.snarkfest.net/compare-talos/index.html?oldRevs=081ebf4e0886, > 3b562fd6d653,7d3a05e286a6&newRev=357af4877abd&submit=true I'll fix the dromaeo regression in place.

Anything left here?

This patch reduced startup resident memory usage by ~2mb on Fennec. \o/

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #35) > Anything left here? No, the remaining work was split off into separate bugs.

Marking fixed per comment 37.

Looking at AreWeSlimYet, on desktop, this reduced startup resident memory by 14mb, which is about 10%. That 14mb reduction entirely eliminates the difference in memory usage we were seeing between startup and 30 seconds later. Pretty great!