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

reducing some bloat centralizing (...args) converstion #10770

Open
WebReflection opened this issue Nov 27, 2019 · 4 comments
Open

reducing some bloat centralizing (...args) converstion #10770

WebReflection opened this issue Nov 27, 2019 · 4 comments

Comments

@WebReflection
Copy link

@WebReflection WebReflection commented Nov 27, 2019

Feature Request

Is your feature request related to a problem? Please describe.
If I write the following:

const a = (...args) => args;
const b = (...args) => args;

each function is bloated by the current arguments to args conversion.

Describe the solution you'd like
As many other parts of Babel are recycled to cover most common transformation, it would be nice to have the same for such common pattern, so that the following:

const a = (...args) => args;
const b = (...args) => args;

would produce this outcome:

"use strict";

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

  return args;
}

var a = function a() {
  return _args2arr.apply(null, arguments);
};

var b = function b() {
  return _args2arr.apply(null, arguments);
};

The minified gzipped size would be 140 bytes, versus 109 bytes with two cases.

However, if we repeat the pattern 10 times, we'll have 165 bytes gzipped (573 bytes uncompressed), which is +18% increase, vs 155 bytes gzipped (944 bytes uncompressed), which is a +42%.

Not only this means the centralized transformer would scale better with bigger projects, but it's important to notice also the non compressed size, which is doubled in the current repeated boilerplate case.

That uncompressed code, is what the engine has to parse, and sparse, always same loops, make the conversion not always easy to promote (in terms of jit/turbofan hot code execution), while a single function that always does the same, might have better chances to be boosted, the code to parse and execute would be smaller, so that everyone should be happy.

Teachability, Documentation, Adoption, Migration Strategy
There is apparently no performance deviation when args2arr.apply(null, arguments) is used, compared to current direct boilerplate fix, so that users should never notice runtime performance degradation, while they might note their code is parsed faster than before, and the overall size of medium to bigger projects reduced, whenever any "thing to array" conversion is needed, as the proposed function would work in a variety of scenarios.

As summary
This is neither urgent nor important, but I'd like to know what are your thoughts about the suggested change.

Thanks in advance for any sort of outcome 👋

@babel-bot

This comment has been minimized.

Copy link
Collaborator

@babel-bot babel-bot commented Nov 27, 2019

Hey @WebReflection! 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."

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Nov 27, 2019

Isn't passing arguments to another function a known deoptimization in old engines? (New engines don't need the transform anyway)

@WebReflection

This comment has been minimized.

Copy link
Author

@WebReflection WebReflection commented Nov 27, 2019

AFAIK only if you pass it without using .apply(null, arguments) ... IIRC .apply(null, arguments) doesn't de-opt, and indeed the two benchmarks show basically no difference in using apply VS direct transform, even in the (...args) => args case

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Nov 27, 2019

Related: #3927, #9152, #211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.