babel / notes

♬ Notes from the @Babel Team (discuss in PRs)
https://github.com/babel/notes#meetings
122 stars 16 forks source link

Discussion: Babel plugin to optimize javascript engine code (like v8) #9

Open hzoo opened 7 years ago

hzoo commented 7 years ago

EDIT: can also be an eslint rule/plugin or a combination of both

Pardon my inexperience/lack of knowledge in all of this but wanted to get some ideas down so we can all learn!

Also ref the thread: https://twitter.com/left_pad/status/814967401276182529 Like in: http://richardartoul.github.io/jekyll/update/2015/04/26/hidden-classes.html

Always instantiate your object properties in the same order so that hidden classes, and subsequently optimized code, can be shared. Adding properties to an object after instantiation will force a hidden class change and slow down any methods that were optimized for the previous hidden class. Instead, assign all of an object’s properties in its constructor. Code that executes the same method repeatedly will run faster than code that executes many different methods only once (due to inline caching).

We should be able to lint/add a runtime check for these?

1  function Point(x,y) {
2    this.x = x;
3    this.y = y;
4  }
5 
7  var obj1 = new Point(1,2);
8
9 obj1.a = 5;

So for the above example we could make a few warnings/errors.

We could warn that a property a is being created on line 9 but the constructor starting on line 1 doesn't contain this.a? So it could be added there, or ask that it be not used if possible?

because

10 obj1.a = 5;
11 obj1.b = 10;
12
13 obj2.b = 10;
14 obj2.a = 5;

have different transition paths we can warn on this kind of thing (moving it to the constructor should fix it as well?)

In some cases we can do a static check for these kinds of things but otherwise we will probably want to just do some code instrumentation around objects/etc to do runtime check/analysis instead?

Maybe we can use something like ES2015 Proxy (or just wrap certain variables in functions), or is it necessary to instrument v8 as well or use something like IRHydra?

Might be cool to see some data around the kinds of objects created (how many times) and what properties are available and added/removed at what lines in the codebase.

Other deopts; https://github.com/petkaantonov/bluebird/wiki/Optimization-killers

cc @addyosmani, @lelandrichardson, @jordwalke

Jessidhia commented 7 years ago

The easiest one to add, probably in babel-eslint, would be warning on delete with a MemberExpression argument ("delete properties"), as it's always a deoptimization.

Objects that are being used like that should be replaced by a Map or Set. Sometimes you do have to do it when interacting with third party code that does use a "hash object", though.

Warning for property add will either require wrapping everything in Proxy, or wrapping every [[Set]] with a function call...

lelandrichardson commented 7 years ago

I'm not sure whether this makes more sense as a babel plugin or an eslint plugin. I feel like eslint might make more sense, since the best thing is probably to let the user know something that they might not know but impacts performance, and let them decide the best way to fix...

Some thoughts on things that might maybe seem catchable by a linter:

  1. using __proto__ object literal
  2. deep logic exit conditions as described here
  3. leaking arguments
  4. any unsafe arguments usage
  5. only optimized for ... in usage
  6. assigning to class properties not assigned to in the constructor
  7. having variables change their backing type (ie, going from int32 to double by having var a = 1; and then later a = 0.5;)
  8. object properties that automatically deopt to a hashtable (ie, var foo = { 'a.b': c };)
  9. using anything unsupported such as debugger, eval, with, etc.
  10. suggest moving larger functions with try { ... } catch () { ... } into a non-try-catched function with a wrapper function calling it from within a try-catch.
Jessidhia commented 7 years ago

@lelandrichardson no. 10 in there could maybe be ignored in async contexts, at least when optimizing for v8. It'll be either in code transpiled into a Promise-based driver (no try/catch), or be executed by Ignition/TurboFan. Or a syntax error 😄

hzoo commented 7 years ago

Either ESLint/Babel works but I figured Babel if this is going to require some kind of runtime changes like instrumenting or transforming code in order to cover certain things.

lelandrichardson commented 7 years ago

Also, If we could actually build this on top of flow, we could have much more insight into the signature of functions, and when they might deopt as a result... Although just writing your JS in strict flow might make it hard to write those deopts in the first place.

xtuc commented 7 years ago

I think this make more sense with an eslint plugin which emits warning. This plugin should focus on optimizations and not best practice.

If we focus on v8 performmance, what about other browsers? We might emits to much warnings for optimizations accross all browsers. A Babel transform plugin makes also sense there since we could just implicitly move stuff.

Aside note: it wouldn't be a surprise to me that v8 would in the future regoranize calls itself as an optimizations. Like other VM does (JVM for example).

Jessidhia commented 7 years ago

All of the "optimization killers" (except for hash table mode objects) should not be a problem anymore with v8 5.5 anyway.

xtuc commented 7 years ago

@Kovensky That's a good news. But there is always legacy stuff sitting around. Could this "optimization killers" also apply to other engines?

kangax commented 7 years ago

@Kovensky is there some kind of reference to find out more about this — why is it no longer relevant? what exactly was changed and how? etc.

bgirard commented 7 years ago

I was actually considering the same thing so I'm glad I found this discussion.

I've written an ESLint warning to match 'delete this.'. This is pretty specific but I think trying to apply this to a large existing code base might produce too many false positives or generally be too noisy and go against developer productivity at the cost of very small performance benefits unless you're in hot code.

I'm leaning that putting this into flow might be the best option. Perhaps letting objects opt into a @monomorphic annotation.

xtuc commented 7 years ago

@bgirard Looks interesting, could you link your repo here?

As a user I wouldn't like having a ton of warning just for the sake of optimization. This should be done implicitly I think. A Babel plugins is actually a better idea IMO.

@hzoo @Kovensky I think this could be a more global subject. As with Babili for minification, what about a Babel project only for engine optimizations.

There are also a lot about parsing performance improvements.

bgirard commented 7 years ago

I don't have a public repo accessible but here's a gist: https://gist.github.com/bgirard/4cd7a2f02f4173a226853bb8b512d09f

It just warns on delete this.<identifier>;. Note that this wont warn on delete this.map[key]; which more likely to be a false positives.

I'm not sure how a babel transformation could make any useful changes statically that would have no observable side-effects. Do you have any ideas?

hzoo commented 7 years ago

@bgirard You can make a Babel transform just error with something like throw path.buildCodeFrameError("Modules aren't supported in the REPL"); but yeah could just be a linting rule instead. I don't think it's about running the babel in production, more of writing code that will error at runtime or statically to let you know in development?