IM is really bad at the following testcase. The problem here is that we don't inline the "new TEst()" call and have to go through the interpreter to run it. That's the observed overhead between IM and JM. Reason: getPolyCallTargets returns 0, because the TypeSet has 1 objCount, that is NULL. // IM: 2.042s // JM: 0.778s // D8: 0.073s var Class = { create: function() { return function() { } } }; var TEst = Class.create(); for (var i=0; i<10000000; i++) { new TEst() }

I thought we were cloning the function such as we can have uniq typesets, but having one object which is set to NULL seems kind of weird. It sounds like the function has been cloned but that we are calling one is not the one used by the interpreter.

So the function indeed get's cloned, but doesn't get an unique type. If the returned function would have "this.initialize.apply(this, arguments);", like in bug #813784, we are smart enough to create new types, because of "UseNewTypeForClone()" in jsinferinlines.h:676 and have a SingleObject. Here we use LCallKnown In the non-singleObject case, we don't have a target and we call LCallConstructor. E.g. resulting in the slowdown.

JM does also invoke interpreter, but can afterwards patch it: [jaeger] PICs stop ignored: first hit (reduced/raytrace/test1.js: 6) [jaeger] PICs getprop patch: inline (/reduced/raytrace/test1.js: 6) [jaeger] PICs patched CALL path 0x7fc023561370 (obj: 0x7fc021a19fc0) The last patching makes the calling as fast as if the target was known (with an extra check if the object is correct) In order to get IM as fast as JM in this case, we will have to do that too

For constructing function we currently always take a VM call. That's why we are so much slower. LCallGeneric is much smarter than LCallConstructor. It was easier to remove LCallConstructor and make LCallGeneric to work with constructing functions. Therefore LCallConstructor is removed in this patch. python jit_test.py --ion $JS/js PASSED ALL time $JS/js test.js 1.027s An increase of 2x on this small testcase. We are still 0.2s slower than JM. I suspect because of the tricks JM applies. E.g. the getprop (I assume the prototype getprop) gets cached. In IM not, because we use "MCallGetProperty". Another note is that the timing with/without constructing is the same for V8! In our engine there is a huge difference with/without constructing. Also our non-constructing speed is almost the same on this benchmark, it is only our constructing speed that is terrible slower!

Why is our constructing performance so much worse?

Comment on attachment 685155 [details] [diff] [review] Use CallGeneric for constructor functions Review of attachment 685155 [details] [diff] [review]: ----------------------------------------------------------------- This sounds good to me, add the comment and r=me. I am adding sstangl in case he has any remarks about the removal of LCallConstructor. As mentionned on IRC, this does not fix the root of the problem which is that TI does not provide a signle type-object after the function is cloned, but this patch still improve by sharing more code and optimize by creating a New this by looking at the prototype of the callee. ::: js/src/ion/CodeGenerator.cpp @@ +904,5 @@ > + > + if (call->mir()->isConstructing()) { > + Label notPrimitive; > + masm.branchTestPrimitive(Assembler::NotEqual, JSReturnOperand, &notPrimitive); > + masm.loadValue(Address(StackPointer, unusedStack), JSReturnOperand); Can you comment what we are reading from the stack and why we erase the return value?

Comment on attachment 685155 [details] [diff] [review] Use CallGeneric for constructor functions Review of attachment 685155 [details] [diff] [review]: ----------------------------------------------------------------- This seems fine. Missing the constructing StackFrame flag may affect debuggers, though. ::: js/src/ion/CodeGenerator.cpp @@ +904,5 @@ > + > + if (call->mir()->isConstructing()) { > + Label notPrimitive; > + masm.branchTestPrimitive(Assembler::NotEqual, JSReturnOperand, &notPrimitive); > + masm.loadValue(Address(StackPointer, unusedStack), JSReturnOperand); I remember this code existing multiple times in LCallKnown. Can it be reused? ::: js/src/ion/IonBuilder.cpp @@ +3571,5 @@ > > MDefinition * > IonBuilder::createThis(HandleFunction target, MDefinition *callee) > { > + if (target) { It would look nicer if we handled the !target case up-top, to remove an indentation level from the bulk of this function, even at the cost of repeating createThisScripted(). I would prefer that this function be left as it was before, with the addition of the !target case. @@ +3791,5 @@ > > MPassArg *thisArg = current->pop()->toPassArg(); > > // If the target is known, inline the constructor on the caller-side. > + if (constructing) { The comment above is now incorrect.

Backed out https://hg.mozilla.org/integration/mozilla-inbound/rev/6b4e13b0d1e4 See bug 16378

(In reply to Hub Figuiere [:hub] from comment #10) > See bug 816378

(In reply to Hub Figuiere [:hub] from comment #10) > Backed out > https://hg.mozilla.org/integration/mozilla-inbound/rev/6b4e13b0d1e4 > > See bug 16378 https://hg.mozilla.org/mozilla-central/rev/6b4e13b0d1e4

This was backed out again, This time because #819299 got backed out for failing on 32bit architectures. Will recommit when other patch gets committed again. 64bit just worked and therefore I have already numbers: - 0.3% increase on kraken, -- 1.1% kraken-astar -- 1.6% kraken-crypto-ccm -- 1.4% kraken-crypto-pbkdf2 -- 2.6% kraken-parse-financial - 0.4% increase on V8 -- 2.8% increase on v8-richards