Bug 706885 adds a bump allocation nursery that works with the interpreter. JIT integration is needed as well. The attached patch is a WIP for this. The main fixes are allocating new objects out of the nursery and adding post write barriers when adding tenured -> nursery edges between objects. This has only barely been tuned for a couple simple examples and in particular the write barriers are as bare bones as possible right now. Also adds minimal JIT integration with JM --- JM will just make a stub call when allocating objects or writing an object to the heap. Fails about 20 jit-tests with --ion-eager --ggc.

New WIP, still fails a few jit-tests --ion-eager --ggc but only for things I think are unrelated to the JIT (weak maps, proxies, ...). Still crashing occasionally on v8bench but usually works; this also adds fixes and optimizations to the object buffer and related code so that we never fall back to a full mark on v8bench. Typical scores I get on v8bench (x86/darwin): trunk (rather, rev 7b3da1dae19a, at the point the ggc stuff was forked of): Richards: 11830 DeltaBlue: 12960 Crypto: 12551 RayTrace: 12210 EarleyBoyer: 13136 RegExp: 1907 Splay: 9290 NavierStokes: 17554 ---- Score (version 7): 9949 ggc: Richards: 10926 DeltaBlue: 15432 Crypto: 12129 RayTrace: 29422 EarleyBoyer: 9907 RegExp: 1725 Splay: 2304 NavierStokes: 17554 ---- Score (version 7): 8961 Notes: - Splay took a massive hit, as expected. Will be fixed after the initial landing by detecting that splay tree nodes and associated data are all being tenured and allocating them directly in the major heap. We can do this by figuring out the objects' allocation sites by looking at their type objects. - Earley-boyer took a big hit rather than the expected big improvement. Main thing that needs investigating. - Deltablue got a nice improvement. Still a little slower than d8, hopefully this one will improve more with tuning or with whatever fixes the earley-boyer issue. - Raytrace got a huge improvement. Part of this is that we aren't doing major GCs and throwing away jitcode now, but even if I add a gcPreserveCode() trunk only improves to 17k. - Other tests don't seem to be affected that much, as expected.

I think this is for all platforms?

Last WIP until bug 706885 lands. This adds various tweaks, not much related to the JIT itself but addressing various performance concerns. - Always trace globals in MarkRuntime, and don't add post barriers for writes to globals. Tons of store buffer entries were being added for global writes in e-b. These could be consolidated but it seems better to avoid them entirely. This needs a mechanism to avoid tracing hundreds of globals in a browser setting but is I think a nice mechanism to have in the general case. - Allow the nursery to grow up to 8MB, dynamically resizing it according to how many objects are being promoted (higher promotion rate ==> larger nursery). e-b wants a big nursery, v8 also grows to 8MB on it. - Don't use generic tracing code for marking during a minor collection, hand optimizing this to avoid overhead improves e-b by about 2000 points. I now get scores with ggc like: Richards: 11074 DeltaBlue: 14235 Crypto: 12926 RayTrace: 29792 EarleyBoyer: 14016 RegExp: 1717 Splay: 3315 NavierStokes: 17554 ---- Score (version 7): 9799 If I transplant in a typical non-ggc score for splay (which we should be able to get to) then the overall score improves to 11146, a 12% improvement over trunk. There is still plenty more perf improvement outside splay to do of course, though that can wait for bug 706885 and the baseline compiler. I think in particular that JM stubbing all operations that can invoke a post barrier hurts these scores significantly.

This is everything in WIP 1 except for (1) what I have already split out into the new blocking bugs, (2) dynamic nursery growth, which I will work on next, and (3) the methodjit bits. I think we should just drop support for JM and focus on getting baseline supported now. There are still a couple FIXME's that need addressing and about 0.5% of the jit-tests are failing. The failures I've investigated so far are due to incorrect barriers on the constant pools and should be easy to address.

Re 3): I assume that merely means, for the moment, that methodjit won't work with GGC, as previously warned?

Comment on attachment 739820 [details] [diff] [review] wip1 Review of attachment 739820 [details] [diff] [review]: ----------------------------------------------------------------- Looks okay to me! ::: js/src/ion/CodeGenerator.cpp @@ +1114,5 @@ > + if (!addOutOfLineCode(ool)) > + return false; > + > + ValueOperand value = ToValue(lir, LPostWriteBarrierV::Input); > + masm.branchTestObject(Assembler::NotEqual, value, ool->rejoin()); Are strings not in the nursery? ::: js/src/ion/IonMacroAssembler.cpp @@ +442,5 @@ > + loadPtr(AbsoluteAddress(nursery.addressOfPosition()), result); > + addPtr(Imm32(thingSize), result); > + branchPtr(Assembler::BelowOrEqual, AbsoluteAddress(nursery.addressOfCurrentEnd()), result, fail); > + storePtr(result, AbsoluteAddress(nursery.addressOfPosition())); > + subPtr(Imm32(thingSize), result); Cool. ::: js/src/ion/LinearScan.cpp @@ +555,5 @@ > // add a torn entry. > if (!safepoint->addNunboxParts(*typeAlloc, *payloadAlloc)) > return false; > + > + if (payloadAlloc->isGeneralReg() && isSpilledAt(payloadInterval, inputOf(ins))) { It's been a long time, so it would be good to have a detailed comment on why this is needed. Is it that... the payload has a canonical spill, but we're only recording the register entry if it exists?

(In reply to David Anderson [:dvander] from comment #7) > Review of attachment 739820 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks okay to me! > > ::: js/src/ion/CodeGenerator.cpp > @@ +1114,5 @@ > > + if (!addOutOfLineCode(ool)) > > + return false; > > + > > + ValueOperand value = ToValue(lir, LPostWriteBarrierV::Input); > > + masm.branchTestObject(Assembler::NotEqual, value, ool->rejoin()); > > Are strings not in the nursery? Not yet: we decided to focus on objects first. String have many places that require the arena header and many places that just steal an internal pointer; we need to deal with these first. > ::: js/src/ion/LinearScan.cpp > @@ +555,5 @@ > > // add a torn entry. > > if (!safepoint->addNunboxParts(*typeAlloc, *payloadAlloc)) > > return false; > > + > > + if (payloadAlloc->isGeneralReg() && isSpilledAt(payloadInterval, inputOf(ins))) { > > It's been a long time, so it would be good to have a detailed comment on why > this is needed. Is it that... the payload has a canonical spill, but we're > only recording the register entry if it exists? Brian?

(In reply to David Anderson [:dvander] from comment #7) > It's been a long time, so it would be good to have a detailed comment on why > this is needed. Is it that... the payload has a canonical spill, but we're > only recording the register entry if it exists? For a given safepoint, LSRA can put the payload for a vreg in up to two places --- the canonical spill location, if it exists, and some register or other location for the vreg's interval covering that location. Both are live, and both need to be recorded in the safepoint. With a non-moving GC only one of the payload locations needs to be recorded.

Current status: jittests --no-jm --no-baseline: 0 failures / 1 timeouts jittests --no-jm --no-baseline --ion-eager: 6 failures / 0 timeouts (parallelarray/scatter[8-13].js except 9) jstests --no-jm --no-baseline: 2 failures / 4 timeouts js1_8_5/regress/regress-595365-2.js js1_8_5/extensions/reflect-parse.js jstests --no-jm --no-baseline --ion-eager: 4 failures / 6 timeouts js1_8_5/regress/regress-595365-2.js js1_8_5/extensions/sps-generators.js js1_8_5/extensions/typedarray.js js1_8_5/extensions/reflect-parse.js I think this is ready for more eyes.

Comment on attachment 748324 [details] [diff] [review] v0 Review of attachment 748324 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/LinearScan.cpp @@ +556,5 @@ > if (!safepoint->addNunboxParts(*typeAlloc, *payloadAlloc)) > return false; > + > + if (payloadAlloc->isGeneralReg() && isSpilledAt(payloadInterval, inputOf(ins))) { > + if (!safepoint->addNunboxParts(*typeAlloc, *payload->canonicalSpill())) nit: please add the comment explaining this

Green try run at: https://tbpl.mozilla.org/?tree=Try&rev=17dcf510b7bb https://hg.mozilla.org/integration/mozilla-inbound/rev/de1c0246854c

Reluctantly backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/b54ce66659aa - one of the four things in that push, despite having been green on try, picked up causing failures in Android 2.2 reftest-1, reftest-2, and reftest-4 by the time it landed. Since I always blame the need to clobber when it's Android, I clobbered and retriggered on your push, and they still failed.

Yeah, incomprehensible is the right word. Naturally, all of those tests are green on my Try push as well.

Yeah, just reland them all again, at a time when you'll be around to argue down anyone who wants to back you out - it cropped up again, someone else got backed out for something else plus this same thing, then it cropped up again after that backout.

Thanks, that's excellent to know! If inserting empty nodes into the MIR graph did actually cause the unrelated code in the image decoder library to stop working, I'd be seriously worried. On the other hand, that implies the image decoder library stops working at random for no reason whatsoever; not exactly comforting.