1383644 - ToObject often not inlined in Speedometer/AngularJS

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[André Bargull [:anba]]

Calls (not inlined) to intrinsic_ToObject [1] in Speedometer (*)

Vanilla         230
Vanilla2015     489
Vanilla-babel   589
React           1115
React-Redux     1197
Ember           5095
Backbone        962
AngularJS       112401
Angular2        691
Vue             873
jQuery          352
Preact          634
Inferno         810
Elm             524
Flight          1584

Apparently we miss to inline ToObject quite often in AngularJS.

Breaking here [2] with 
    br MCallOptimize.cpp:2906 if callInfo.args_.mBegin[0]->resultType_!=js::jit::MIRType::Object
and 
    br MCallOptimize.cpp:2906 if callInfo.args_.mBegin[0]->resultType_==js::jit::MIRType::Value

we can measure how often ToObject isn't inlined because Arg[0] isn't an object.

ToObject       All       Arg[0] not Object
React-Redux     21       2
Ember           87       0
Backbone         6       0
AngularJS       58      16      <-- Only because callInfo.getArg(0)->type() == MIRType::Value
Angular2         8       0
Vue              6       0
jquery           3       0
Preact           7       0
Inferno         18       0
Elm              7       0
Flight          19       0

I guess we should handle MIRType::Value when it flows into the ToObject call given that it is the second most called intrinsic per bug 1365361?


(*) Measured via gdb breakpoints, so the numbers may not be accurate to the last digit :-)
[1] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/js/src/vm/SelfHosting.cpp#82
[2] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/js/src/jit/MCallOptimize.cpp#2906
[3] http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/js/src/jit/MCallOptimize.cpp#2915

Comment 1

[André Bargull [:anba]]

(In reply to André Bargull from comment #0)
> Calls (not inlined) to intrinsic_ToObject [1] in Speedometer (*)
> 
> Vanilla         230
> Vanilla2015     489
> Vanilla-babel   589
> React           1115
> React-Redux     1197
> Ember           5095
> Backbone        962
> AngularJS       112401
> Angular2        691
> Vue             873
> jQuery          352
> Preact          634
> Inferno         810
> Elm             524
> Flight          1584
> 
> Apparently we miss to inline ToObject quite often in AngularJS.
> 

With the patch to inline ToObject with MIRType::Value as the input, the non-inlined calls to ToObject drop to 3000-5000 for AngularJS.

Comment 2

[André Bargull [:anba]]

Attachment: bug1383644.patch

This is similar to the existing MToObjectOrNull, except we can't make it moveable, because we always need to return a new object when ToObject is called with a primitive value and it'd be observable to return the same primitive wrapper multiple times.

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8892002 [details] [diff] [review]
bug1383644.patch

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

Inline all the things :)

::: js/src/jit/MCallOptimize.cpp
@@ +2947,5 @@
> +        current->push(object);
> +    } else {
> +        auto* ins = MToObject::New(alloc(), object);
> +        current->add(ins);
> +        current->push(ins);

Here (the else-branch) it would also be good to use pushTypeBarrier because it's likely more precise than any-object.

Comment 4

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8997b7d045fca1be174dfedb8d14aa563e09a8bf

Comment 5

[André Bargull [:anba]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> Comment on attachment 8892002 [details] [diff] [review]
> bug1383644.patch
> 
> Review of attachment 8892002 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Inline all the things :)
> 

And one day :djvj will complain he can no longer use his scripts from bug 1357180/1365361 to sample built-ins. ;-)

Comment 6

[André Bargull [:anba]]

Attachment: bug1383644.patch

Updated patch per review comments, carrying r+.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4b029e2a35d
Inline ToObject when called with MIRType::Value. r=jandem

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f4b029e2a35d