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

ES2015 default parameters: arguments performance issue #5922

Closed
ghost opened this issue Jul 6, 2017 · 4 comments
Closed

ES2015 default parameters: arguments performance issue #5922

ghost opened this issue Jul 6, 2017 · 4 comments
Labels
i: duplicate outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@ghost
Copy link

ghost commented Jul 6, 2017

Hi guys,
Thank you very much for all your work on babel, I believe front-end development wouldn't be where it is today without it.
So, currently babel transpiles es2015 default parameters by making use of the arguments variable, which has performance drawbacks.

Input Code

REPL link available :)

function add(a = 0, b = 1) {
  return a + b;
}

Babel Configuration (.babelrc, package.json, cli command)

{
  presets: [
    ["es2015", {"loose": true}],
  ],
}

Expected Behavior

"use strict";

function add(aArg, bArg) {
  var a = aArg || 0;
  var b = bArg || 1;

  return a + b;
}

Current Behavior

"use strict";

function add() {
  var a = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : 0;
  var b = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : 1;

  return a + b;
}

Possible Solution

Default parameters have historically been managed like in the "Expected behaviour" section, which is fast, I don't know much about ASTs in general so I'm not sure how trivial this is to change.
I think there might be solutions that still make use of arguments, depending on what's easier to implement, but I'd avoid it if possible.
This is one, but I'm not sure of its perf impacts, so we'll have to test it.

Context

I had been using babel's default parameters blindly and one of our functions ended up in "hot" code, which made me discover the perf problem and opening this issue.
I know that @GoodBoyDigital and @bigtimebuddy had to stop using them in some parts of PixiJs due to this problem, happy to hear your thoughts guys if you have anything to add from your perf tests.

Your Environment

software version(s)
Babel 6.25.0
node v8.0.0
npm 5.0.4
Operating System MacOSX 10.11.6

Thank you,
Alvin

@babel-bot
Copy link
Collaborator

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

@jridgewell
Copy link
Member

I'm closing this as a duplicate of #5922, since it'd solve this completely.

@loganfsmyth
Copy link
Member

@jridgewell Looks like you linked to the wrong issue.

@jridgewell
Copy link
Member

Lololololol, meant #5776.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 4, 2018
@lock lock bot locked as resolved and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
i: duplicate outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

3 participants