Skip to content

Replace _.each and for-in with for loop in hot paths#432

Merged
sebmck merged 3 commits intobabel:masterfrom
gaearon:perf-stable
Jan 10, 2015
Merged

Replace _.each and for-in with for loop in hot paths#432
sebmck merged 3 commits intobabel:masterfrom
gaearon:perf-stable

Conversation

@gaearon
Copy link
Copy Markdown
Member

@gaearon gaearon commented Jan 9, 2015

This is my first PR extracted from experiments in #428.
This alone gives me 24% time improvement (from 212ms on my benchmark to 160ms).

@gaearon
Copy link
Copy Markdown
Member Author

gaearon commented Jan 9, 2015

With second commit, 29% improvement over master (150ms instead of 212ms).

@chicoxyzzy
Copy link
Copy Markdown
Member

👍 looks good

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it makes a difference, but does caching the length here help? i.e. for (var i = 0, length = str.length; i < length; i++) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it doesn't appear so. I ran it and it's the same speed.
I remember reading V8 does that optimization by itself.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I just saw the discussion on #428.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK caching built-in length won't make big difference in modern JS engine like V8. See: http://mrale.ph/blog/2014/12/24/array-length-caching.html

@eventualbuddha
Copy link
Copy Markdown
Member

👍

@gaearon
Copy link
Copy Markdown
Member Author

gaearon commented Jan 9, 2015

I added a few minor impovements: gaearon@5783973

In my testing, they shave off about 5-10ms from my benchmark.

I'm afraid I can't find other low-hanging optimization fruit other than this and #433 so I'll be glad if people smarter than me continued with this. :-)

@jamiebuilds
Copy link
Copy Markdown
Contributor

👍

sebmck added a commit that referenced this pull request Jan 10, 2015
Replace _.each and for-in with for loop in hot paths
@sebmck sebmck merged commit 508b353 into babel:master Jan 10, 2015
@sebmck
Copy link
Copy Markdown
Contributor

sebmck commented Jan 10, 2015

Awesome, thanks for this!

@gaearon gaearon deleted the perf-stable branch January 10, 2015 10:58
@sebmck sebmck mentioned this pull request Jan 13, 2015
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 8, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 8, 2019
kaicataldo pushed a commit that referenced this pull request Nov 14, 2019
* chore(package): update eslint-config-babel to version 6.0.0

Closes #432

https://greenkeeper.io/

* Fix linting
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants