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

Various performance improvements #3

Merged
merged 2 commits into from
Jun 27, 2014
Merged

Conversation

Krinkle
Copy link
Contributor

@Krinkle Krinkle commented Jun 27, 2014

About 30% performance improvement (from 170ms to 120ms).

"(implementation" -> "(implementation)"
…loops

About 30% performance improvement (from 170ms to 120ms).

* Use native Array.isArray instead of comparing [[Class]] via toString.
  Seems to reliably save about 10ms in performance, and makes more sense
  in general.

* Cache hasOwnProperty.
  Instead of requesting a global variable and two property levels inside
  the loop, cache it once.

* Don't use a for-in loop over an array.
  For-in loops are for objects, not arrays. They scan through the
  entire inheritance chain (thus needing a hasOwn check) and they
  potentially yield keys in the wrong order (ironic, since this
  code is all about avoiding internal sort and using alphabetical
  instead).

Performance (average of 10 runs):

- Before (2167f41)
  - native: 46-48ms
  - js stringify with js sort: 165-173ms
  - native stringify with js sort: 170-176ms
- After
  - native: 47-53ms
  - js stringify with js sort: 165-171ms
  - native stringify with js sort: 124-129ms
@Krinkle
Copy link
Contributor Author

Krinkle commented Jun 27, 2014

I squashed the performance tweaks down to one commit, but the separate ones are there if want (some) of them:

  • 5bd9c3e - index2: Don't use a for-in loop over an array
  • b86ea76 - index2: Cache hasOwnProperty
  • 9a4c64a - index2: Use native Array.isArray instead of comparing [[Class]] via toString

mirkokiefer added a commit that referenced this pull request Jun 27, 2014
Various performance improvements
@mirkokiefer mirkokiefer merged commit a599648 into mirkokiefer:master Jun 27, 2014
@mirkokiefer
Copy link
Owner

Nice, thank you!

@Krinkle Krinkle deleted the cleanup branch June 27, 2014 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants