Closed Bug 846259 Opened 10 years ago Closed 10 years ago

BaselineCompiler: Avoid script/pc lookup in fallback stubs

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Currently many of the fallback stubs call GetTopIonJSScript to get the top JSScript. This is slow and in many cases not necessary.

The micro-benchmark below takes 467 ms with --no-ion. If I just pass the CalleeToken to the BinaryArith fallback stub, it takes 360 ms.

(In this case we could also attach a stub that handles everything inline, but there will always be cases we don't optimize.)

function f() {
    for (var i=0; i<10000000; i++) {
	i | undefined;
    }
}
var t = new Date;
f();
print(new Date - t);
Seems like a good plan.  Will probably help SS out a bunch, as well as improving scores overall.
Attached patch PatchSplinter Review
Initially I tried to push the CalleeToken or JSScript directly, but that's a bit complicated with eval frames (callee token is the outer function/script and evalScript is the script iself). This patch just passes the BaselineFrame * and the stubs can get the script from that. Also allows us to get rid of some GetTopBaselineFrame calls.
Attachment #720672 - Flags: review?(kvijayan)
Comment on attachment 720672 [details] [diff] [review]
Patch

Review of attachment 720672 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/BaselineIC.cpp
@@ +1527,5 @@
>              bool lhsIsUndefined = lhs.isNull() || lhs.isUndefined();
>              bool compareWithNull = lhs.isNull() || rhs.isNull();
>              ICCompare_ObjectWithUndefined::Compiler compiler(cx, op,
>                                                               lhsIsUndefined, compareWithNull);
> +            ICStub *objectStub = compiler.getStub(compiler.getStubSpace(frame->script()));

Here and above, the expression |compiler.getStubSpace(frame->script())| is showing up a lot.  It's not an expensive operation.  It might make things look cleaner to get the stub space at the beginning of the function and just use it alter.

@@ +1813,5 @@
>      }
>  
>      if (arg.isNull() || arg.isUndefined()) {
>          ICToBool_NullUndefined::Compiler compiler(cx);
> +        ICStub *nilStub = compiler.getStub(compiler.getStubSpace(frame->script()));

Same comment as above.
Attachment #720672 - Flags: review?(kvijayan) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/72a31d6bfa65
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.