1370208 - Inline Boolean when called as a function

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement)

Description

[André Bargull [:anba]]

I saw this in a Speedometer/React profile and it should be easy enough to inline in MCallOptimize using IonBuilder::convertToBoolean(...).

Comment 1

[Nicolas B. Pierron [:nbp]]

(In reply to André Bargull from comment #0)
> I saw this in a Speedometer/React profile and it should be easy enough to
> inline in MCallOptimize using IonBuilder::convertToBoolean(...).

Can you provide more information, with ideally a micro benchmark which highlight the issue?

Comment 2

[André Bargull [:anba]]

    var q = 0;
    var inputs = [true, false, 0, 1, "", "awd", null, {}];
    var t = Date.now();
    for (var i = 0; i < 10000000; ++i) {
        if (Boolean(inputs[i & 0x7])) q++;
    }
    print(Date.now() - t, q);

Would improve from 125ms to ~30ms (same number as V8, which also optimizes calls to the Boolean constructor -> https://github.com/v8/v8/blob/a234136074366a8ab8d1c703bad1949ec5a1f033/src/compiler/js-call-reducer.cc#L67-L81).

FWIW in Speedometer, this React function seems to cause most Boolean calls: https://github.com/facebook/react/blob/0.14-stable/src/renderers/shared/reconciler/ReactCompositeComponent.js#L689

Comment 3

[André Bargull [:anba]]

Attachment: bug1370208.patch

The patch used for comment #2.

Do you think this makes sense to optimize?

Comment 4

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8874444 [details] [diff] [review]
bug1370208.patch

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

::: js/src/jit/MCallOptimize.cpp
@@ +904,5 @@
>  
>  IonBuilder::InliningResult
> +IonBuilder::inlineBoolean(CallInfo& callInfo)
> +{
> +    if (callInfo.argc() != 1 || callInfo.constructing()) {

nit/follow-up: It seems that the 0 argument case, or more than one case should be easy to implement as well.

http://searchfox.org/mozilla-central/source/js/src/jsbool.cpp#116

Comment 5

[André Bargull [:anba]]

Attachment: bug1370208.patch

Relaxed arguments check to allow only number of arguments per comment #4. Carrying r+.

Comment 6

[André Bargull [:anba]]

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f302f3481aaa
Inline Boolean constructor when called as a function. r=nbp

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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