babel / babel

🐠 Babel is a compiler for writing next generation JavaScript.
https://babel.dev
MIT License
43.19k stars 5.64k forks source link

plugin-transform-async-to-generator could create more efficient code #15921

Open overlookmotel opened 1 year ago

overlookmotel commented 1 year ago

💻

What problem are you trying to solve?

I'd like to suggest a change to babel-plugin-transform-async-to-generator which would produce (I believe) faster code for methods which contain super.

Input:

class C extends S {
  async foo(x) {
    await super.foo(x);
  }
}

Current output with babel-plugin-transform-async-to-generator:

class C extends S {
  foo(x) {
    var _superprop_getFoo = () => super.foo,
      _this = this;
    return _asyncToGenerator(function* () {
      yield _superprop_getFoo().call(_this, x);
    })();
  }
}

REPL

_asyncToGenerator() is called every time foo() is called.

Describe the solution you'd like

This could instead be transformed to:

var _foo = _asyncToGenerator(function*(_this, _superprop_getFoo, x) {
  yield _superprop_getFoo().call(_this, x);
});
class C extends S {
  foo(x) {
    return _foo(this, () => super.foo, x);
  }
}

I think this is functionally equivalent, but _asyncToGenerator() is only called once when the class is created, rather than on every call to foo().``

Describe alternatives you've considered

_asyncToGenerator could be memoized. But I think suggested change above is simpler, and more efficient.

Documentation, Adoption, Migration Strategy

No response

babel-bot commented 1 year ago

Hey @overlookmotel! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

overlookmotel commented 1 year ago

NB: A reference to the class name inside the class is a different binding from externally. So in this case:

class C extends S {
  async foo() {
    await super.foo();
    return C;
  }
}
const C2 = C;
C = 123;

C would need to be passed into the generator function so it gets the right C:

var _foo = _asyncToGenerator(function*(_this, _superprop_getFoo, C) {
  yield _superprop_getFoo().call(_this);
  return C;
});
class C extends S {
  foo() {
    return _foo(this, () => super.foo, C);
  }
}
const C2 = C;
C = 123;
liuxingbaoyu commented 1 year ago

In this case, arguments are changed. It does not matter. Inspired by you, maybe we can do this?

var c_foo;
class C extends S {
  foo(x) {
    return (c_foo = c_foo || _asyncToGenerator(function* (_this, _superprop_getFoo) {
      yield _superprop_getFoo().call(_this, x);
    }))(this, () => super.foo);
  }
}
liuxingbaoyu commented 1 year ago

I investigated asyncToGenerator and found that this may have a small impact on performance. However I found that asyncToGenerator can significantly reduce the size. :)

overlookmotel commented 1 year ago

@liuxingbaoyu Ah that's really nice! Yes, your version is better than what I came up with.

I guess c_foo ||= _asyncToGenerator() would be even shorter if the target supports that syntax.