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

split/join vs replace #39

Merged
1 commit merged into from
Sep 28, 2010
Merged

split/join vs replace #39

1 commit merged into from
Sep 28, 2010

Conversation

ryantenney
Copy link
Contributor

Replace .split().join() in _.template as per discussion on fbd682d and benchmarks here: http://jsperf.com/split-join-vs-replace

@jashkenas
Copy link
Owner

Thanks for the patch. That's really quite an amazing difference -- merged to master.

@ryantenney
Copy link
Contributor Author

Thanks for the merge!

"Foolish is the man who attempts to benchmark JavaScript performance in Chrome"

  • Me (circa just a moment ago)

Call me a skeptic, but I had a sneaking suspicion that when run in Chrome the benchmark was just spitting out ridiculous numbers thanks to some static analysis and various other voodoo spells and incantations. So I attempted to de-tune the benchmarks in such a way that Chrome wouldn't be able to decide that a particular block of code has no side effect and "optimize" it out of existence. What I ended up with were some more credible numbers that show a substantially more modest improvement of anywhere from 1 - 20% in Chrome and more impressive numbers (5 - 100+%) in other modern browsers. (updated benchmarks here: http://jsperf.com/split-join-vs-replace/2)

Lesson learned.

@jashkenas
Copy link
Owner

Uh oh, with your new benchmark, it's showing about 2x slower performance for replace() in Safari on the larger string sizes ... perhaps we patched it a bit too soon.

@ryantenney
Copy link
Contributor Author

I haven't had a similar result, at least not in Safari 5. Unfortunately I don't have any old browsers installed (Safari 5 is the only non-beta browser I have installed).

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants