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

turn transform into a simple for loop #3477

Merged
merged 1 commit into from
Apr 24, 2016
Merged

Conversation

mattkrick
Copy link
Contributor

I'm all for expressive code, but the transform function is HOT. Profiling my webpack build, the function accounts for ~20% of the ticks. This is verified through my webpack build times: I reduced my build from ~12 seconds to ~10 seconds.
Before:

After:

I'm not saying we should do this for every forEach, but a 20% reduction in build times just for changing a loop type seems like an easy win.

@codecov-io
Copy link

Current coverage is 85.81%

Merging #3477 into master will not affect coverage as of 431e77c

@@            master   #3477   diff @@
======================================
  Files          197     197       
  Stmts        12227   12228     +1
  Branches      2521    2521       
  Methods          0       0       
======================================
  Hit          10493   10493       
- Partial        522     523     +1
  Missed        1212    1212       

Review entire Coverage Diff as of 431e77c

Powered by Codecov. Updated on successful CI builds.

@loganfsmyth loganfsmyth merged commit 27bd5c6 into babel:master Apr 24, 2016
@loganfsmyth
Copy link
Member

Thanks! I'm surprised it was that much of a difference. Are you using passPerPreset?

@hzoo hzoo added area: perf PR: Internal 🏠 A type of pull request used for our changelog categories labels Apr 24, 2016
@milesj
Copy link

milesj commented Apr 25, 2016

Native forEach() is notoriously slower than for loops, so this isn't really surprising. This could be a quick win if it was changed in areas of the code that are hit the most.

@loganfsmyth
Copy link
Member

Totally, but I would not have expected the top level transform function to be called enough to matter, hence my surprise.

@hzoo
Copy link
Member

hzoo commented Apr 25, 2016

We could also use for (let pluginPass of (this.pluginPasses: Array)) {} to take advantage of our Array type optimization which transpiles to a regular loop which is in rest of the codebase

@mattkrick
Copy link
Contributor Author

@loganfsmyth nah, just a real standard config. I bet it'd do even better for those folks using passPerPreset.
I was really surprised, too, which is why I included the results (I wouldn't believe it if I didn't run it myself).

@mattkrick mattkrick deleted the patch-1 branch April 25, 2016 18:36
@Macil
Copy link
Contributor

Macil commented Jun 8, 2016

@hzoo Are you sure that works? An optimization like that sounds handy but I don't observe that doing anything special in the output code, even in building Babel itself.

@hzoo
Copy link
Member

hzoo commented Jun 8, 2016

@agentme you can take that convo to #3485 (and as logan mentioned it's not special for babel since we already use loose mode - only makes the output simpler.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: perf outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants