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

React/Flow/Type optimisations #653

Closed
kittens opened this Issue Feb 1, 2015 · 31 comments

Comments

Projects
None yet
@kittens
Member

kittens commented Feb 1, 2015

As per #568 I've been thinking of areas where static analysis and JavaScript transformation can really shine and one area in particular that I've thought of is the optimisation of React components. There's a lot of areas for possible optimisations, especially with the use of flow types and React propTypes.

For example consider the following:

var Foo = React.createClass({
  propTypes: {
    items: React.PropTypes.array
  },

  render() {
    return <div>{this.props.items.map(function (item) {
      return <p>{item.title}</p>;
    })}</div>;
  }
});

This would normally be compiled to:

var Foo = React.createClass({
  displayName: "Foo",

  propTypes: {
    items: React.PropTypes.array
  },

  render: function render() {
    return React.createElement(
      "div",
      null,
      this.props.items.map(function (item) {
        return React.createElement("p", null, item.title);
      })
    );
  }
});

However with the optimisations I'm proposing it would instead compile to:

var Foo = React.createClass({
  displayName: "Foo",

  propTypes: {
    items: React.PropTypes.array
  },

  render: function render() {
    return React.createElement(
      "div",
      null,
      (function (items) {
        var children = [];
        for (var i = 0; i < items.length; i++) {
          var item = items[i];
          children.push(React.createElement("p", null, item.title));
        }
        return children;
      })(this.props.items)
    );
  }
});

It may even be possible to drop the IIFE depending on the order of evaluation. There's a lot of patterns that could be handled to ensure that the code is as fast as possible. Other array methods such as filter, some, every etc could also be handled.

I think this would be extremely beneficial especially in hot methods such as render. You could even extend this to inlining methods so:

var Button = React.createClass({
  title() {
    return (
      <div className="detail">
        <h2 className="detail__title">{this.props.options.title}</h2>
      </div>
    );
  },

  render() {
    return (
      <div className="dialog__inner">
        {this.title()}
      </div>
    );
  }
});

would just become:

var Button = React.createClass({
  displayName: "Button",

  render: function render() {
    return React.createElement(
      "div",
      { className: "dialog__inner" },
      React.createElement(
        "div",
        { className: "detail" },
        React.createElement(
          "h2",
          { className: "detail__title" },
          this.props.options.title
        )
      )
    );
  }
});

Reference tracking would also need to be implemented to handle the following:

var props = this.props; // register that `props` is `this.props`
props.foo;

var foo = this.props.foo; // register that `foo` is `this.props.foo`
var foo2 = foo; // register that `foo2` is `this.props.foo`
foo = bar; // `foo` has been dereferenced

/cc @RReverser @amasad @fkling @fabiomcosta @leebyron

There's a lot that can be done and I'd be interested to hear your feedback on whether something like this is valuable and specific problem areas that could be optimised with static analysis.

@kittens kittens added the discussion label Feb 1, 2015

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 1, 2015

Member

These optimisations aren't really React-specific and extend to Flow types etc.

Member

kittens commented Feb 1, 2015

These optimisations aren't really React-specific and extend to Flow types etc.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Feb 1, 2015

Member

That's interesting. This has to be opt-in because propTypes don't have runtime semantics (they only validate stuff in dev).

I'm sure @sebmarkbage, @chenglou, @spicyj and @syranide have ideas about potential optimizations.

One potential optimization I can think of is reusing ReactElements that we know don't change (see “Optimizations” in this gist):

Since it's now the responsibility of the consumer to create a ReactElement efficiently, that opens up a lot of opportunities for us to do optimizations in the JS transforms that may be dependent on how the consumer uses them. For example inline objects, pooling and reusing constant objects.

var Button = require('Button'); // mock
var sameEveryTime = { type: Button, props: { prop: "foo" } };
class App {
  render() {
    return sameEveryTime;
  }
}

Maybe @swannodette can also shine some Om wisdom on what we can do here.

Member

gaearon commented Feb 1, 2015

That's interesting. This has to be opt-in because propTypes don't have runtime semantics (they only validate stuff in dev).

I'm sure @sebmarkbage, @chenglou, @spicyj and @syranide have ideas about potential optimizations.

One potential optimization I can think of is reusing ReactElements that we know don't change (see “Optimizations” in this gist):

Since it's now the responsibility of the consumer to create a ReactElement efficiently, that opens up a lot of opportunities for us to do optimizations in the JS transforms that may be dependent on how the consumer uses them. For example inline objects, pooling and reusing constant objects.

var Button = require('Button'); // mock
var sameEveryTime = { type: Button, props: { prop: "foo" } };
class App {
  render() {
    return sameEveryTime;
  }
}

Maybe @swannodette can also shine some Om wisdom on what we can do here.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 1, 2015

Member

@gaearon Yep, it'd be opt-in for sure. There's potentially a performance penalty associated and I don't want to force this kind of code mangling on users by default. It'd be through an optional transformer so to enable it would just be: $ 6to5 --optional optimisation.react.

Member

kittens commented Feb 1, 2015

@gaearon Yep, it'd be opt-in for sure. There's potentially a performance penalty associated and I don't want to force this kind of code mangling on users by default. It'd be through an optional transformer so to enable it would just be: $ 6to5 --optional optimisation.react.

@leebyron

This comment has been minimized.

Show comment
Hide comment
@leebyron

leebyron Feb 1, 2015

Contributor

I'm not quite sure I understand what you're proposing. What does this have to do with React? Your example shows the inlining of an array map operation and inlining a function call. Is the React part just that reading propTypes can yield some type information?

Also, are you sure these are actually optimizations? This seems pretty dangerous as you can't guarantee the code will behave the same since it's up to the implementations of map/filter, and the replacements will have different VM optimization outcomes.

IMHO, this seems like a pretty fragile suggestion given the dynamic nature of JS.

Contributor

leebyron commented Feb 1, 2015

I'm not quite sure I understand what you're proposing. What does this have to do with React? Your example shows the inlining of an array map operation and inlining a function call. Is the React part just that reading propTypes can yield some type information?

Also, are you sure these are actually optimizations? This seems pretty dangerous as you can't guarantee the code will behave the same since it's up to the implementations of map/filter, and the replacements will have different VM optimization outcomes.

IMHO, this seems like a pretty fragile suggestion given the dynamic nature of JS.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Feb 1, 2015

This proposal is quite ambitious. It's not clear that you would actually see much benefit after a few iterations in a good JIT. It also relies on type analysis to change runtime behavior which is fragile at best and might cause serious bugs.

There are much more low-hanging fruit that we would like to formally specify.

For example, in 0.13 we're pseudo-freezing the props object which will allow us to make it immutable in future versions. If we also treat JSX as referentially transparent objects, we can reuse constant instances.

After getting rid of some meta data we can also start inlining object literals. We wouldn't want to do this in development mode since we'd want propType validation and key validation. But a production mode compiler could inline it.

On Feb 1, 2015, at 1:32 PM, Sebastian McKenzie notifications@github.com wrote:

@gaearon Yep, it'd be opt-in for sure. There's potentially a performance penalty associated and I don't want to force this kind of code mangling on users by default. It'd be through an optional transformer so to enable it would just be: $ 6to5 --optional optimisation.react.


Reply to this email directly or view it on GitHub.

sebmarkbage commented Feb 1, 2015

This proposal is quite ambitious. It's not clear that you would actually see much benefit after a few iterations in a good JIT. It also relies on type analysis to change runtime behavior which is fragile at best and might cause serious bugs.

There are much more low-hanging fruit that we would like to formally specify.

For example, in 0.13 we're pseudo-freezing the props object which will allow us to make it immutable in future versions. If we also treat JSX as referentially transparent objects, we can reuse constant instances.

After getting rid of some meta data we can also start inlining object literals. We wouldn't want to do this in development mode since we'd want propType validation and key validation. But a production mode compiler could inline it.

On Feb 1, 2015, at 1:32 PM, Sebastian McKenzie notifications@github.com wrote:

@gaearon Yep, it'd be opt-in for sure. There's potentially a performance penalty associated and I don't want to force this kind of code mangling on users by default. It'd be through an optional transformer so to enable it would just be: $ 6to5 --optional optimisation.react.


Reply to this email directly or view it on GitHub.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 1, 2015

Member

@leebyron

I'm not quite sure I understand what you're proposing. What does this have to do with React? Your example shows the inlining of an array map operation and inlining a function call. Is the React part just that reading propTypes can yield some type information?

Yep, which is why I followed up by saying "These optimisations aren't really React-specific and extend to Flow types etc.".

Also, are you sure these are actually optimizations? This seems pretty dangerous as you can't guarantee the code will behave the same since it's up to the implementations of map/filter, and the replacements will have different VM optimization outcomes.

Sure, except we can be more lenient. map and forEach always have to keep sparse arrays etc in mind and there's no real way for the engine to opt out of that because there's no type information available to it.

With the following prop type { arrayOf: React.PropTypes.arrayOf(React.PropTypes.number) } you can effectively ignore a lot of the aforementioned behaviour.

IMHO, this seems like a pretty fragile suggestion given the dynamic nature of JS.

Sure, which is why they'd never be performed unless you were 100% confident.


It's probably important to note that I started this issue to discuss it as I'm sure there are areas that I'm not considering. I'm not at all suggesting that it's practical which is why I reached out for feedback.

Member

kittens commented Feb 1, 2015

@leebyron

I'm not quite sure I understand what you're proposing. What does this have to do with React? Your example shows the inlining of an array map operation and inlining a function call. Is the React part just that reading propTypes can yield some type information?

Yep, which is why I followed up by saying "These optimisations aren't really React-specific and extend to Flow types etc.".

Also, are you sure these are actually optimizations? This seems pretty dangerous as you can't guarantee the code will behave the same since it's up to the implementations of map/filter, and the replacements will have different VM optimization outcomes.

Sure, except we can be more lenient. map and forEach always have to keep sparse arrays etc in mind and there's no real way for the engine to opt out of that because there's no type information available to it.

With the following prop type { arrayOf: React.PropTypes.arrayOf(React.PropTypes.number) } you can effectively ignore a lot of the aforementioned behaviour.

IMHO, this seems like a pretty fragile suggestion given the dynamic nature of JS.

Sure, which is why they'd never be performed unless you were 100% confident.


It's probably important to note that I started this issue to discuss it as I'm sure there are areas that I'm not considering. I'm not at all suggesting that it's practical which is why I reached out for feedback.

@RnbWd

This comment has been minimized.

Show comment
Hide comment
@RnbWd

RnbWd Feb 2, 2015

I'm not sure if this is related, and I'm watching the sb halftime show its crazy haha

I built an app in react ~11 / immutable ~ 2, and it broke into an infinite loop that took a day to debug. React was updated to 12.2, immutable stayed at ~2, but 6to5 ~latest compiled everything. In the end, react 12.2 components transformed inside immutable ~2 using react ~11 api caused an infinite loop. Which was just weird, but it may be related to this thread in some way.

RnbWd commented Feb 2, 2015

I'm not sure if this is related, and I'm watching the sb halftime show its crazy haha

I built an app in react ~11 / immutable ~ 2, and it broke into an infinite loop that took a day to debug. React was updated to 12.2, immutable stayed at ~2, but 6to5 ~latest compiled everything. In the end, react 12.2 components transformed inside immutable ~2 using react ~11 api caused an infinite loop. Which was just weird, but it may be related to this thread in some way.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 2, 2015

Member

@RnbWd It's definently not, this is a discussion thread, nothing has actually been done yet.

Member

kittens commented Feb 2, 2015

@RnbWd It's definently not, this is a discussion thread, nothing has actually been done yet.

@chenglou

This comment has been minimized.

Show comment
Hide comment
@chenglou

chenglou Feb 2, 2015

Not much to contribute =)

(If you feel really hardcore, you can look into Haskell's stream fusion and see how to eliminate intermediate arrays completely by rewriting map/filter/reduce using loops.)

chenglou commented Feb 2, 2015

Not much to contribute =)

(If you feel really hardcore, you can look into Haskell's stream fusion and see how to eliminate intermediate arrays completely by rewriting map/filter/reduce using loops.)

@RReverser

This comment has been minimized.

Show comment
Hide comment
@RReverser

RReverser Feb 2, 2015

Member

(If you feel really hardcore, you can look into Haskell's stream fusion and see how to eliminate intermediate arrays completely by rewriting map/filter/reduce using loops.)

Or just extract ready-to-use lazy functions from new Lo-dash :P

Member

RReverser commented Feb 2, 2015

(If you feel really hardcore, you can look into Haskell's stream fusion and see how to eliminate intermediate arrays completely by rewriting map/filter/reduce using loops.)

Or just extract ready-to-use lazy functions from new Lo-dash :P

@kittens kittens changed the title from React optimisations to React/Flow/Type optimisations Feb 2, 2015

@RnbWd

This comment has been minimized.

Show comment
Hide comment
@RnbWd

RnbWd Feb 2, 2015

To clarify my edge case:

Using normal arrays to map/filter/reduce my components - the application worked fine. It's when I did map/filter/reduce with React components inside of immutable collections that I ran into the problem. I was using an outdated version of Immutable, so I expected some errors. But this particular issue gave no warnings, and completely froze the browser.

I follow these libraries closely in my feed, and I'm updating some older projects to the current versions, and this discussion thread happens to mesh everything together. The problem / solution was just so obscure / hard to debug I felt the need to mention it somewhere.

RnbWd commented Feb 2, 2015

To clarify my edge case:

Using normal arrays to map/filter/reduce my components - the application worked fine. It's when I did map/filter/reduce with React components inside of immutable collections that I ran into the problem. I was using an outdated version of Immutable, so I expected some errors. But this particular issue gave no warnings, and completely froze the browser.

I follow these libraries closely in my feed, and I'm updating some older projects to the current versions, and this discussion thread happens to mesh everything together. The problem / solution was just so obscure / hard to debug I felt the need to mention it somewhere.

@swannodette

This comment has been minimized.

Show comment
Hide comment
@swannodette

swannodette Feb 2, 2015

I'm with @leebyron and @sebmarkbage, I don't really see the point. In all my benchmarking with React I've never once seen any time be dominated by maps, filters, reduces, or any immutable data structure. All the significant time is spent in React reconciliation. I would do some rigorous benchmarking before pursuing micro-optimizations such as this one.

swannodette commented Feb 2, 2015

I'm with @leebyron and @sebmarkbage, I don't really see the point. In all my benchmarking with React I've never once seen any time be dominated by maps, filters, reduces, or any immutable data structure. All the significant time is spent in React reconciliation. I would do some rigorous benchmarking before pursuing micro-optimizations such as this one.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 2, 2015

Member

@swannodette The array optimisation was just an example, I'm not at all implying that it's the most practical or sane.

Everyone seems to be focussing on my specific (bad) examples. While my original question was if there was any common patterns that could potentially lead to significant optimisations.

Member

kittens commented Feb 2, 2015

@swannodette The array optimisation was just an example, I'm not at all implying that it's the most practical or sane.

Everyone seems to be focussing on my specific (bad) examples. While my original question was if there was any common patterns that could potentially lead to significant optimisations.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Feb 2, 2015

Member

Can we implement inlining like this with current version of React? Is this beneficial?

I'm talking about turning React.createElement calls into object literals, and extracting those that don't depend on variables.

Member

gaearon commented Feb 2, 2015

Can we implement inlining like this with current version of React? Is this beneficial?

I'm talking about turning React.createElement calls into object literals, and extracting those that don't depend on variables.

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Feb 2, 2015

Currently, they all depend on internals that are not exposed. We need to continue with the context/owner refactoring in React to make it work.

sebmarkbage commented Feb 2, 2015

Currently, they all depend on internals that are not exposed. We need to continue with the context/owner refactoring in React to make it work.

@phpnode

This comment has been minimized.

Show comment
Hide comment
@phpnode

phpnode Feb 2, 2015

Contributor

It seems like if you want to do this kind of optimization (rewriting .map to a loop etc), catering specifically to React would be a mistake. Flow types are the obvious way to approach this, it's a real shame that flow isn't a JS library itself - to do this safely 6to5 needs a way to reliably infer types. If type information was available in the AST it'd be a lot easier to plugin many different types of optimisation, e.g. tail calls etc.

Contributor

phpnode commented Feb 2, 2015

It seems like if you want to do this kind of optimization (rewriting .map to a loop etc), catering specifically to React would be a mistake. Flow types are the obvious way to approach this, it's a real shame that flow isn't a JS library itself - to do this safely 6to5 needs a way to reliably infer types. If type information was available in the AST it'd be a lot easier to plugin many different types of optimisation, e.g. tail calls etc.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 2, 2015

Member

@phpnode Flow type information is available in the AST.

Member

kittens commented Feb 2, 2015

@phpnode Flow type information is available in the AST.

@phpnode

This comment has been minimized.

Show comment
Hide comment
@phpnode

phpnode Feb 2, 2015

Contributor

@sebmck afaik it doesn't propogate... or does it? e.g. presumably

function foo (x: string) {
  let y = x.toUpperCase();
  // what is y's type?
}
Contributor

phpnode commented Feb 2, 2015

@sebmck afaik it doesn't propogate... or does it? e.g. presumably

function foo (x: string) {
  let y = x.toUpperCase();
  // what is y's type?
}
@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 2, 2015

Member

It doesn't but inferring the type of y in that specific scenario would be
trivial.

On Tuesday, 3 February 2015, Charles Pick notifications@github.com wrote:

@sebmck https://github.com/sebmck afaik it doesn't propogate... or does
it? e.g. presumably

function foo (x: string) {
let y = x.toUpperCase();
// what is y's type?
}

Reply to this email directly or view it on GitHub
#653 (comment).

Sebastian McKenzie

Member

kittens commented Feb 2, 2015

It doesn't but inferring the type of y in that specific scenario would be
trivial.

On Tuesday, 3 February 2015, Charles Pick notifications@github.com wrote:

@sebmck https://github.com/sebmck afaik it doesn't propogate... or does
it? e.g. presumably

function foo (x: string) {
let y = x.toUpperCase();
// what is y's type?
}

Reply to this email directly or view it on GitHub
#653 (comment).

Sebastian McKenzie

@jeffmo

This comment has been minimized.

Show comment
Hide comment
@jeffmo

jeffmo Feb 3, 2015

Contributor

Normally inferring types doesn't lend itself to such simple scenarios. Many cases get trickier.

For example:

function getFoo(x: Array) {
  return x.map(function(item) {
    return item + 42;
  });
}

class MyArrayClass extends Array {}
class MySortaArrayClass extends Array {
  map(cb) {
    // does something completely different from A.p.map
  }
}

var y = getFoo(new MyArrayClass());
// y's exact type is MyClass -- but that's a subtype of Array. it's map() method
// could be inlined, but you have a lot of tracking to do to verify that to be true

var z = getFoo(new MySortaArrayClass());
// z's exact type is MySortArrayClass -- a subtype of Array, but one whose 
// map method can't be inlined the same way as an array map() method might

Indeed you could stick to the simple cases if you wanted, but it's worth pointing out that the Flow type information that sits in the AST are really just hints to a fairly complex algorithm that propagates them. In some cases the propagation is simple; but in most it's not.

At the moment Flow doesn't have a way of exporting the results of tracking these propagations. It is on our roadmap for some point in the future, but it's a tricky problem in and of itself so probably not for a while. Once that information is extractable you might be able to take it and do more interesting things based on the type of some arbitrary expression as calculated by Flow.

Realistically, for now, I think you'll find that you're pretty limited in what kinds of optimizations you can do safely here (even with only validated typehints).

Contributor

jeffmo commented Feb 3, 2015

Normally inferring types doesn't lend itself to such simple scenarios. Many cases get trickier.

For example:

function getFoo(x: Array) {
  return x.map(function(item) {
    return item + 42;
  });
}

class MyArrayClass extends Array {}
class MySortaArrayClass extends Array {
  map(cb) {
    // does something completely different from A.p.map
  }
}

var y = getFoo(new MyArrayClass());
// y's exact type is MyClass -- but that's a subtype of Array. it's map() method
// could be inlined, but you have a lot of tracking to do to verify that to be true

var z = getFoo(new MySortaArrayClass());
// z's exact type is MySortArrayClass -- a subtype of Array, but one whose 
// map method can't be inlined the same way as an array map() method might

Indeed you could stick to the simple cases if you wanted, but it's worth pointing out that the Flow type information that sits in the AST are really just hints to a fairly complex algorithm that propagates them. In some cases the propagation is simple; but in most it's not.

At the moment Flow doesn't have a way of exporting the results of tracking these propagations. It is on our roadmap for some point in the future, but it's a tricky problem in and of itself so probably not for a while. Once that information is extractable you might be able to take it and do more interesting things based on the type of some arbitrary expression as calculated by Flow.

Realistically, for now, I think you'll find that you're pretty limited in what kinds of optimizations you can do safely here (even with only validated typehints).

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 3, 2015

Member

@jeffmo Yeah, was just pointing out that @phpnode's example is something that could be handled relatively easily. I don't think the fact that you can't easily handle the more complex scenarios is a reason not to investigate and perform some of the more simple optimisations that you can do with 100% confidence.

@amasad actually bought up flow outputting type information here which is what initially got me thinking of things that could be done sooner rather than later. Once there's infrastructure in place to perform these simple optimisations it'll make it easier to integrate the more complex ones once flow allows it.

Member

kittens commented Feb 3, 2015

@jeffmo Yeah, was just pointing out that @phpnode's example is something that could be handled relatively easily. I don't think the fact that you can't easily handle the more complex scenarios is a reason not to investigate and perform some of the more simple optimisations that you can do with 100% confidence.

@amasad actually bought up flow outputting type information here which is what initially got me thinking of things that could be done sooner rather than later. Once there's infrastructure in place to perform these simple optimisations it'll make it easier to integrate the more complex ones once flow allows it.

@isiahmeadows

This comment has been minimized.

Show comment
Hide comment
@isiahmeadows

isiahmeadows Feb 5, 2015

Contributor

I left an explanation of some of the difficulties in type analysis in a simple case within an UglifyJS2 feature request. This specific case deals with objects and analyzing types with those, but the concepts can extend to other areas. Just a little canary.

It'll get the most complicated with Flow types.

Contributor

isiahmeadows commented Feb 5, 2015

I left an explanation of some of the difficulties in type analysis in a simple case within an UglifyJS2 feature request. This specific case deals with objects and analyzing types with those, but the concepts can extend to other areas. Just a little canary.

It'll get the most complicated with Flow types.

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 5, 2015

Member

@impinball Yeah, this is more of a long term goal if anything. I've already started doing some work with inferring types in some very simple cases. Obviously you can't do anything extremely complex without something sophisticated like flow.

I want to make some of these simple transformations possible which is inline with the project scope and future that I outlined in #568. Adding minification transformers is completely within scope and I think the structure of 6to5 lends extremely well to that as you can easily abstract out transformers rather than having everything in a single file which I find completely unacceptable. I know that didn't address your specific points but just wanted to elaborate on my intentions as I feel like they've been pretty foggy so far.

Member

kittens commented Feb 5, 2015

@impinball Yeah, this is more of a long term goal if anything. I've already started doing some work with inferring types in some very simple cases. Obviously you can't do anything extremely complex without something sophisticated like flow.

I want to make some of these simple transformations possible which is inline with the project scope and future that I outlined in #568. Adding minification transformers is completely within scope and I think the structure of 6to5 lends extremely well to that as you can easily abstract out transformers rather than having everything in a single file which I find completely unacceptable. I know that didn't address your specific points but just wanted to elaborate on my intentions as I feel like they've been pretty foggy so far.

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Feb 11, 2015

Thanks for considering this!

People who think the JIT takes care of all these things miss the part where things like .filter(..).map are still and have been really slow in JS for a while now IMO. With type information 6to5 can certainly help the JIT by producing much faster code IMO

benjamingr commented Feb 11, 2015

Thanks for considering this!

People who think the JIT takes care of all these things miss the part where things like .filter(..).map are still and have been really slow in JS for a while now IMO. With type information 6to5 can certainly help the JIT by producing much faster code IMO

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Feb 12, 2015

Member

Related #757.

Destructuring/spreading a known variable that has been annotated as an Array will inline it instead of wrapping it in a helper to ensure that it's an array. ie:

var arr: Array = bar;
[...arr];

Will turn into:

var arr = bar;
[].concat(arr);

instead of:

"use strict";

var _toConsumableArray = ...;

var arr = bar;
[].concat(_toConsumableArray(arr));

Similarly this happens with rest parameters:

function foo(...items) {
  return [...items];
}

Becomes:

"use strict";

function foo() {
  for (var _len = arguments.length, items = Array(_len), _key = 0; _key < _len; _key++) {
    items[_key] = arguments[_key];
  }

  return [].concat(items);
}

(Note: items is inlined as we internally know that it's an Array as the es6.properties.rest transformer registers it as one)

Pretty happy with this as no other transpiler does this type of type checking and optimisation.

Member

kittens commented Feb 12, 2015

Related #757.

Destructuring/spreading a known variable that has been annotated as an Array will inline it instead of wrapping it in a helper to ensure that it's an array. ie:

var arr: Array = bar;
[...arr];

Will turn into:

var arr = bar;
[].concat(arr);

instead of:

"use strict";

var _toConsumableArray = ...;

var arr = bar;
[].concat(_toConsumableArray(arr));

Similarly this happens with rest parameters:

function foo(...items) {
  return [...items];
}

Becomes:

"use strict";

function foo() {
  for (var _len = arguments.length, items = Array(_len), _key = 0; _key < _len; _key++) {
    items[_key] = arguments[_key];
  }

  return [].concat(items);
}

(Note: items is inlined as we internally know that it's an Array as the es6.properties.rest transformer registers it as one)

Pretty happy with this as no other transpiler does this type of type checking and optimisation.

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Feb 12, 2015

@sebmck you might want to talk to @petkaantonov about ideas for these optimizations - bluebird has a build step that does such things (for examples slicing).

I think that actually going faster with 6to5 than without it would be incredible :D

benjamingr commented Feb 12, 2015

@sebmck you might want to talk to @petkaantonov about ideas for these optimizations - bluebird has a build step that does such things (for examples slicing).

I think that actually going faster with 6to5 than without it would be incredible :D

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Mar 6, 2015

Member

I've since made some internal architectural changes related to AST path representations that would allow a type inference engine to be plugged in fairly easily (whether via some data dumped by Flow or even integrate the TypeScript engine somehow).

And with facebook/react/issues/3226 and facebook/react#3228 I'm satisfied that I'll be able to do some of the type of optimisations I set out to enquire about with this issue in the first place. Thanks everyone!

Member

kittens commented Mar 6, 2015

I've since made some internal architectural changes related to AST path representations that would allow a type inference engine to be plugged in fairly easily (whether via some data dumped by Flow or even integrate the TypeScript engine somehow).

And with facebook/react/issues/3226 and facebook/react#3228 I'm satisfied that I'll be able to do some of the type of optimisations I set out to enquire about with this issue in the first place. Thanks everyone!

@kittens kittens closed this Mar 6, 2015

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Mar 12, 2015

Member

Going to reopen this since I'm doing some more work on resolving flow types. I'm going to post periodic updates here so apologies to people subscribed to this issue who aren't interested, feel free to unsubscribe.

Also just a fore note but just so I'm clear I'm not trying to replicate flow type checking and inference (that would be absurd), I'm only resolving them for some simple cases.


The es6.forOf transformer now takes arrays into consideration and will turn it into a for (var i = 0; i < arr.length; i++) which is the fastest way to iterate over an array. The resolution is only local to the file and it does not follow imports.

The following:

// inline
for (var num of [1, 2, 3]) {}

// inferred - the inference will be stripped if `nums` is reassigned
var nums = [1, 2, 3]
for (var num of nums) {}

// declarator type
var nums: Array = [1, 2, 3];
for (var num of nums) {}

// function return type
function foo(): Array {}
for (var num of foo()) {}

// method in object with return type
var bar = { foo(): Array {} };
for (var num of bar.foo()) {}

// nested method in object with return type
var bar = { bar: { foo(): Array {} } };
for (var num of bar.bar.foo()) {}

is turned into:

// inline
var _arr = [1, 2, 3];
for (var _i = 0; _i < _arr.length; _i++) {
  var num = _arr[_i];
}

// inferred - the inference will be stripped if `nums` is reassigned
var nums = [1, 2, 3];
for (var _i = 0; _i < nums.length; _i++) {
  var num = nums[_i];
}

// declarator type
var nums = [1, 2, 3];
for (var _i = 0; _i < nums.length; _i++) {
  var num = nums[_i];
}

// function return type
function foo() {}
var _arr = foo();
for (var _i = 0; _i < _arr.length; _i++) {
  var num = _arr[_i];
}

// method in object with return type
var bar = { foo: function foo() {} };
var _arr = bar.foo();
for (var _i = 0; _i < _arr.length; _i++) {
  var num = _arr[_i];
}

// nested method in object with return type
var bar = { bar: { foo: function foo() {} } };
var _arr = bar.bar.foo();
for (var _i = 0; _i < _arr.length; _i++) {
  var num = _arr[_i];
}

After the path abstraction I mentioned in my previous comment it made it super easy to check if a path was of a certain type.

export function ForOfStatement(node, parent, scope, file) {
  if (this.get("right").isTypeGeneric("Array")) {
    // for..in logic here
  }
}

This is all being done in the experimental branch and will eventually be merged for the next major (behind a flag most likely too) so I have time to optimise the actual type resolution and iron out bugs such as reassignments etc before release.

Member

kittens commented Mar 12, 2015

Going to reopen this since I'm doing some more work on resolving flow types. I'm going to post periodic updates here so apologies to people subscribed to this issue who aren't interested, feel free to unsubscribe.

Also just a fore note but just so I'm clear I'm not trying to replicate flow type checking and inference (that would be absurd), I'm only resolving them for some simple cases.


The es6.forOf transformer now takes arrays into consideration and will turn it into a for (var i = 0; i < arr.length; i++) which is the fastest way to iterate over an array. The resolution is only local to the file and it does not follow imports.

The following:

// inline
for (var num of [1, 2, 3]) {}

// inferred - the inference will be stripped if `nums` is reassigned
var nums = [1, 2, 3]
for (var num of nums) {}

// declarator type
var nums: Array = [1, 2, 3];
for (var num of nums) {}

// function return type
function foo(): Array {}
for (var num of foo()) {}

// method in object with return type
var bar = { foo(): Array {} };
for (var num of bar.foo()) {}

// nested method in object with return type
var bar = { bar: { foo(): Array {} } };
for (var num of bar.bar.foo()) {}

is turned into:

// inline
var _arr = [1, 2, 3];
for (var _i = 0; _i < _arr.length; _i++) {
  var num = _arr[_i];
}

// inferred - the inference will be stripped if `nums` is reassigned
var nums = [1, 2, 3];
for (var _i = 0; _i < nums.length; _i++) {
  var num = nums[_i];
}

// declarator type
var nums = [1, 2, 3];
for (var _i = 0; _i < nums.length; _i++) {
  var num = nums[_i];
}

// function return type
function foo() {}
var _arr = foo();
for (var _i = 0; _i < _arr.length; _i++) {
  var num = _arr[_i];
}

// method in object with return type
var bar = { foo: function foo() {} };
var _arr = bar.foo();
for (var _i = 0; _i < _arr.length; _i++) {
  var num = _arr[_i];
}

// nested method in object with return type
var bar = { bar: { foo: function foo() {} } };
var _arr = bar.bar.foo();
for (var _i = 0; _i < _arr.length; _i++) {
  var num = _arr[_i];
}

After the path abstraction I mentioned in my previous comment it made it super easy to check if a path was of a certain type.

export function ForOfStatement(node, parent, scope, file) {
  if (this.get("right").isTypeGeneric("Array")) {
    // for..in logic here
  }
}

This is all being done in the experimental branch and will eventually be merged for the next major (behind a flag most likely too) so I have time to optimise the actual type resolution and iron out bugs such as reassignments etc before release.

@kittens kittens reopened this Mar 12, 2015

kittens added a commit that referenced this issue Mar 12, 2015

separate binding logic from scope to a binding class, move binding ty…
…pe resolution to the path so it can be used on any expression - #653
@cpojer

This comment has been minimized.

Show comment
Hide comment
@cpojer

cpojer Mar 12, 2015

Member

@sebmck I think this particular transform is going to be a little more troublesome. If you infer the type of something as Array<number> there is no way to know whether it is a subclass of Array that uses a different iterator. Imagine something like this:

class ReverseArray extends Array {}
ReverseArray[Symbol.iterator] = MyReverseArrayIterator;

Now I'm not saying this example is great, but it can be done using ES6 and having a transform trying to guess this optimization is probably not ideal :)

Member

cpojer commented Mar 12, 2015

@sebmck I think this particular transform is going to be a little more troublesome. If you infer the type of something as Array<number> there is no way to know whether it is a subclass of Array that uses a different iterator. Imagine something like this:

class ReverseArray extends Array {}
ReverseArray[Symbol.iterator] = MyReverseArrayIterator;

Now I'm not saying this example is great, but it can be done using ES6 and having a transform trying to guess this optimization is probably not ideal :)

@kittens

This comment has been minimized.

Show comment
Hide comment
@kittens

kittens Mar 13, 2015

Member

@cpojer Yeah that was a point raised by @jeffmo. I'll put it behind an optional transformer called optimisation.flow.forOf or something like that and create some docs similar to loose mode where all the caveats and pitfalls can be clearly documented so you can enable it if you're adamant that your app wont run into them. I mainly just did it as a proof of concept to prove that Babel was flexible enough and because it was super fun figuring out how to do it.

Member

kittens commented Mar 13, 2015

@cpojer Yeah that was a point raised by @jeffmo. I'll put it behind an optional transformer called optimisation.flow.forOf or something like that and create some docs similar to loose mode where all the caveats and pitfalls can be clearly documented so you can enable it if you're adamant that your app wont run into them. I mainly just did it as a proof of concept to prove that Babel was flexible enough and because it was super fun figuring out how to do it.

@kittens kittens closed this Mar 29, 2015

@lock lock bot added the outdated label Jul 23, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Jul 23, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.