Prepack: Referentialization should not introduce unnecessary array accesses

Created on 15 Jun 2017  路  4Comments  路  Source: facebook/prepack

Prepacking this

(function(){
    let f = function(x){
        return function() {
            for (let i=0; i<100; i++) x++;
            return x;
        }
    }
    global.g = [f(2), f(6)];
})();

currently results in the following.

(function () {
    var __captured_scopes = Array(2);
    function $_0(__scope_1) {
        if (!__captured_scopes[__scope_1]) __captured_scopes[__scope_1] = {
            x: 2
        };
        for (let i = 0; i < 100; i++) __captured_scopes[__scope_1].x++;
        return __captured_scopes[__scope_1].x;
    }

    function $_1(__scope_2) {
        if (!__captured_scopes[__scope_2]) __captured_scopes[__scope_2] = {
            x: 6
        };
        for (let i = 0; i < 100; i++) __captured_scopes[__scope_2].x++;
        return __captured_scopes[__scope_2].x;
    }

    var _1 = $_0.bind(null, 0);
    var _2 = $_1.bind(null, 1);
    g = [_1, _2];
})();

There are various inefficiencies here, but let's focus on the following:

  • In the loop, there are repeated lookups to __captured_scopes[__scope_?]. This should be avoided.

Instead, Prepack should generate the following:

(function () {
    var __captured_scopes = Array(2);
    function $_0(__scope_1) {
        let __captured_scope = __captured_scopes[__scope_1];
        if (!__captured_scope) __captured_scopes[__scope_1] = __captured_scope = {
            x: 2
        };
        for (let i = 0; i < 100; i++) __captured_scope.x++;
        return __captured_scope.x;
    }

    function $_1(__scope_2) {
        let __captured_scope = __captured_scopes[__scope_2];
        if (!__captured_scope) __captured_scopes[__scope_2] = __captured_scope = {
            x: 6
        };
        for (let i = 0; i < 100; i++) __captured_scope.x++;
        return __captured_scope.x;
    }

    var _1 = $_0.bind(null, 0);
    var _2 = $_1.bind(null, 1);
    g = [_1, _2];
})();
enhancement help wanted serializer

Most helpful comment

I'm going to dive into this issue

All 4 comments

I'm going to dive into this issue

@NTillmann I've tried to figure out how things work under the hood but it's not easy to get my hand dirty. If I could have your advice where I should start to tackle this issue, that must be super helpful.

In ResidualFunctions.js, there are three places right now where the expressions like __captured_scopes[__scope_2] are build up via t.memberExpression(this.capturedScopesArray, t.identifier(scope.name), true). Those are the places where we need to do some tweaking.

In _getReferentializedScopeInitialization, before emitting the if statement, we should emit a let statement as outlined above. Instead of hard-coding a local variable name such as __captured_scope, the name of the new local variable could be mangled from the scope.name in case there's more than one relevant scope.

Then in _referentialize, you would replace the member expression with a reference to the corresponding local variable.

Addressed by merged pull request.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

vojtechkral picture vojtechkral  路  5Comments

sebmarkbage picture sebmarkbage  路  6Comments

sandipp picture sandipp  路  4Comments

simon-yxl picture simon-yxl  路  3Comments

obenjiro picture obenjiro  路  4Comments