Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add lambda lifting optimize command #265

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

nyuichi
Copy link
Member

@nyuichi nyuichi commented Sep 10, 2013

Sorry but this is WORK-IN-PROGRESS, I want some comments or reviews on this implementation strategy.

sample code:

class _Main {
    static function main (args : string[]) : void {
        function square (n : number) : number {
            return n * n;
        }
        log square(2);
    }
}

$ bin/jsx --optimize lambda-lifting foo.jsx

$__jsx_extend([_Main], Object);
function _Main$main$AS(args) {
    console.log(_Main$square$N(2));
};

_Main.main = _Main$main$AS;
_Main.main$AS = _Main$main$AS;

function _Main$square$N(n) {
    return n * n;
};

_Main.square$N = _Main$square$N;

As of now the command optimizes first-level (not nested) closures in method by lifting them up to the method level. This is the same strategy as "staticize" optimize command (actually I borrowed a plenty of code from that).

My future plans are

  1. Optimize nested inner function statements as well.
  2. If they contain "this" inside themselves, do lift them up to the block level (the top statement block in the method) instead of staticize.
  3. finally do it against function expressions, too.

@ghost ghost assigned nyuichi Sep 18, 2013
@kazuho
Copy link
Member

kazuho commented Dec 4, 2013

The idea sounds fine. And I am not against adding experimental optimization subcommands to the master branch.

Looking at the code I could not find any analysis of escapes that return or set the reference to the closure somewhere (in which case the closure should not be removed). Is there any work done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants