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

BlockBindingTransform should not use try/catch always #6

Closed
arv opened this issue Mar 28, 2013 · 13 comments
Closed

BlockBindingTransform should not use try/catch always #6

arv opened this issue Mar 28, 2013 · 13 comments

Comments

@arv
Copy link
Collaborator

arv commented Mar 28, 2013

Original author: jmesserly@google.com (April 15, 2011 22:37:37)

Currently BlockBinding is always generated with try/catch. We should try to use functions when possible.

Original issue: http://code.google.com/p/traceur-compiler/issues/detail?id=3

@arv
Copy link
Collaborator Author

arv commented Mar 28, 2013

From arv@chromium.org on June 23, 2012 20:30:08
Another thing we could do is to detect when we don't need a let and change it to a var.

I think if the following are true we could change it to a var

  1. Not captured by any closure
  2. Not redefined inside another block
  3. Not referenced in ancestor block.

I think 2 and 3 can be achieved by renaming the binding

@arv
Copy link
Collaborator Author

arv commented Mar 28, 2013

From edy.b...@gmail.com on January 18, 2013 06:12:54
I recall people talking about bad performance within try/catch in v8, and me doing tests proving that (performance wasn't affected in called functions though).

I think there's a v8 issue for this, and I also remember them saying something on the lines of "we could probably enable some optimizations, but right now we have none inside try/catch".

I doubt it's the first time you hear about it, so here's my 4th option: add an option to generate let keywords in the output, as SpiderMonkey and v8 (with some flags) support it.
Provided that the engine's implementation is compatible, it should speed things up.

@arv
Copy link
Collaborator Author

arv commented Mar 28, 2013

From edy.b...@gmail.com on January 19, 2013 11:49:45
Also, #1 should not only include closures, but eval. Then again, eval'ing JS code within traceur output is not a good idea, and could be quite broken already.

@arv
Copy link
Collaborator Author

arv commented Mar 28, 2013

From arv@chromium.org on January 20, 2013 18:28:45
edy.burt: You can do the following:

traceur.options.blockBinding = 'parse';

Most Traceur options are tri state (true, false and 'parse'). When the option is set to 'parse' the feature is parsed but not transformed.

If you find any bugs related to "traceur.options.blockBinding = 'parse'" please file a new bug since this is definitely something we want to have fully working.

@arv
Copy link
Collaborator Author

arv commented Mar 28, 2013

From olov.las...@gmail.com on February 07, 2013 08:02:22
Regarding comment #1, would you mind showing an example of why var-renaming (instead of try/catch) doesn't work when a variable is captured by a closure? Thanks.

@arv
Copy link
Collaborator Author

arv commented Mar 28, 2013

From arv@google.com on February 07, 2013 15:18:02
olov: The classic problem with vars in loops is one

var fs = [];
for (var i = 0; i < 10; i++) {
let x = i;
fs.push(function() { return x; });
}

maybe this is limited to loops? I can't think of any non loop case.

@arv
Copy link
Collaborator Author

arv commented Mar 28, 2013

From olov.las...@gmail.com on February 07, 2013 20:17:38
Thanks - that one slipped my mind. If that's the only case (can't come up with any other one either) then it should mean that you can get rid of the vast majority of your generated try/catches, all of them even depending on what your preferred transformation is.

@eddyb
Copy link
Contributor

eddyb commented May 9, 2013

From #297:

Looking at the way traceur transforms for with a let declaration, I see the write-back that requires a try block itself.
Wouldn't it be possible to look for assignments to loop variables and use try-finally for write-back only if a write-back can occur?

Since for loops with let declarations are the main usecase of block-bindings, this could speed things up.
Then again, if you're using try-catch for block-bindings, the code would still be wrapped, even without the write-back.

@eddyb
Copy link
Contributor

eddyb commented May 25, 2013

Another idea would be to replace assignments to the iterator so they assign both the scoped iterator and the original one. That won't require the try-finally block at all.

@mgol
Copy link

mgol commented May 27, 2014

Is this something hard to do? AngularJS 2 currently doesn't use let & const because of this performance penalty (that's why @IgorMinar reported #1036), we'd need sth working like @olov's defs in most cases for it to be feasible.

@arv
Copy link
Collaborator Author

arv commented May 28, 2014

This is not that hard. It is easier than some of the transformations we already do and harder than others. It is just a matter of lack of time from my side.

@dead-claudia
Copy link

Out of curiosity, would a function wrapper with all the vars declared at the top above any of them be better? Or, in a more concrete sense, this?

function foo() {
  var bar = 2;
  let array = []
  for (let i of x) {
    let {prop} = i;
    array.push(prop + bar);
  }
  try {
    prop == null;
  } catch (e) {
    return array;
  }
  throw new Error();
}

// to...

function foo() {
  var bar = 2;
  var array = []; // wrapping this is useless
  for (var i$ in x) {
    (function (i) { // copy index, now it's local to the loop
      var prop = i.prop; // wrapping this is useless
      array.push(prop + bar);
    })(x[i$]);
  }
  try {
    prop == null; // should throw
  } catch (e) {
    return array;
  }
  throw new Error();
}

I know this example is relatively let-specific, but const should be similar as long as it is verified no zero direct assignment occurs.

@arv
Copy link
Collaborator Author

arv commented Sep 6, 2014

@IMPinball That is pretty much what we are doing now:

function f() {
  var fs = [];
  for (var i = 0; i < 10; i++) {
    let x = i;
    fs[x] = () => x;
  }

  x;  // throws ReferenceError: x is not defined
}
f()

Generates:

function f() {
  var fs = [];
  var $__104 = function(i) {
    var x = i;
    fs[x] = (function() {
      return x;
    });
  };
  for (var i$__103 = 0; i$__103 < 10; i$__103++) {
    $__104(i$__103);
  }
  x;
}
f();

Closing this in favor of #1277

@arv arv closed this as completed Sep 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants