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

Faster if a little more verbose case insensitive 'on' match #275

Merged
merged 1 commit into from Aug 18, 2016

Conversation

c2h5oh
Copy link
Contributor

@c2h5oh c2h5oh commented Aug 18, 2016

See: https://jsfiddle.net/c2h5oh/rr9b6Leb/1/

string.match: 3,418,862 ops/sec ±2.79%
char compare: 38,104,121 ops/sec ±2.67%

@developit
Copy link
Member

Indeed! This was actually how it used to work. I switched it out because it made the build a bit smaller, but maybe the runtime performance is more important? Also, I'm not sure how much people care about case-insensitive on prefix for events. Maybe we just go back to the old solution, which is more or less what you have here. Here's the commit where I changed it:

5c30d4c

Thanks for bringing this up though, I really shouldn't have just merged that in without checking with people first!

BTW - I had an esbench with roughly the same test as yours, managed to find it:
https://esbench.com/bench/574c954bdb965b9a00965ac6

Saves writing the Benchmark.js setup (it's still using that benchmark.js behind the scenes)

What do you think? Maybe we go back to case-sensitive, since nobody ever complained about it? 😋

@developit developit merged commit f7956ff into preactjs:master Aug 18, 2016
@developit
Copy link
Member

I merged and then switched back to the case-sensitive version. You were right about the performance, .match() had started to show up in my profiling and that was why. I think we can just roll with lowercase on for now as a happy medium between performance and usefulness 👍

@c2h5oh
Copy link
Contributor Author

c2h5oh commented Aug 18, 2016

Indeed! This was actually how it used to work. [..] Here's the commit where I changed it:

That's how I noticed the change - reading recent commits

@developit
Copy link
Member

Cool - thanks for this btw, nice to have other people keeping tabs on decisions, haha!

@developit developit added this to the 5.7 milestone Aug 18, 2016
@c2h5oh
Copy link
Contributor Author

c2h5oh commented Aug 19, 2016

So I explored this a little bit: added more possible solutions to benchmark. Still using benchmark.js, because esbench made browser throw slow script warnings on a larger number of tests and async just didn't work.

https://jsfiddle.net/c2h5oh/84q4ema6/

Using str[0] = 'o' && str[1] as baseline (100%) this is what I got in Chrome 51.0.2704.84:

What Speed change
str[0] === 'o' && str[1] === 'n' 100%
str.match(/^on/) -90.1%
/^on/.text(str) -88.0%
str.indexOf('on') === 0 -86.4%
str.slice(0,2) === 'on' -65.3%
str.substring(0,2) === 'on' -66.9%

slice is the 2nd fastest (by 2/3..) and length-wise somewhere in the middle of char comparison and match.

When I re-run the benchmark in Firefox 48 baseline was roughly 75% faster than Chrome and that was just the start - things got way more interesting after:

What Speed change
str[0] === 'o' && str[1] === 'n' 100%
str.match(/^on/) -95.9%
/^on/.text(str) -97.7%
str.indexOf('on') === 0 -86.4%
str.slice(0,2) === 'on' +2237%
str.substring(0,2) === 'on' +498%

Slice is 23+ times faster than char comparison (which is already faster than in chrome) and substring almost 6 times..

@developit
Copy link
Member

Thinking Firefox is doing something fancy with short-circuiting the comparison... maybe because the boolean value is never being used? It seems a little bit crazy to me that creating a new String on every iteration would be faster than checking the value at a given offset. Perhaps .slice() caches in some explicit way in Firefox...

Regarding esbench - the async checkbox is for an async test, which requires calling deferred.resolve() on completion. Slow script warnings in Firefox is a known thing, I can't seem to figure out why Benchmark.js isn't respecting {async:true} :(

@developit
Copy link
Member

developit commented Aug 19, 2016

@c2h5oh hmm - I very slightly modified the tests by setting the outcome of the comparisons to an enclosed variable: https://jsfiddle.net/developit/84q4ema6/6/

With that in place, I'm seeing compare as the fastest by quite a bit in both Chrome and Firefox:

Chrome

3.5x faster than slice (which is the next best)

Chrome

Firefox:

24.5x faster than slice (also the next best here)

Firefox

-edit- btw, thanks a ton for digging into this. Helps make sure decisions are rooted in fact! 🎯

@c2h5oh
Copy link
Contributor Author

c2h5oh commented Aug 19, 2016

Chrome:
string.test x 6,396,455 ops/sec ±1.77% (62 runs sampled)
string.match x 7,144,616 ops/sec ±0.85% (65 runs sampled)
compare x 61,874,254 ops/sec ±1.14% (66 runs sampled)
indexOf x 8,455,225 ops/sec ±0.24% (65 runs sampled)
slice x 20,736,922 ops/sec ±0.77% (65 runs sampled)
substring x 20,574,470 ops/sec ±0.79% (65 runs sampled)

Firefox:
string.test x 2,992,271 ops/sec ±3.24% (38 runs sampled)
string.match x 6,534,201 ops/sec ±0.23% (39 runs sampled)
compare x 110,410,806 ops/sec ±0.61% (40 runs sampled)
indexOf x 15,570,957 ops/sec ±0.28% (40 runs sampled)
slice x 2,457,230,954 ops/sec ±0.98% (36 runs sampled)
substring x 613,989,844 ops/sec ±2.79% (30 runs sampled)

@developit
Copy link
Member

developit commented Aug 19, 2016

@c2h5oh is that with the updated jsfiddle? What version of Firefox and what OS? The performance of JS engines varies pretty wildly by OS.

@c2h5oh
Copy link
Contributor Author

c2h5oh commented Aug 19, 2016

Yes, updated jsfiddle

➜  ~ uname -a
Linux 4.6.2-040602-generic

➜  ~ google-chrome --version
Google Chrome 51.0.2704.84

➜  ~ firefox --version
Mozilla Firefox 48.0

@developit
Copy link
Member

Hmm. Perhaps this is an OS X oddity then... I'm on OS X 10.10.5 with roughly the same chrome and firefox versions.

@developit
Copy link
Member

Ran the fiddle on IE10 and Chrome under Windows (via SauceLabs), seems to be favoring compare as well:

I'm sortof inclined to be timid and stick with the direct compare here since Firefox on Linux seems to be the outlier. Seem reasonable?

@c2h5oh
Copy link
Contributor Author

c2h5oh commented Aug 19, 2016

On Windows 10, same computer.

Chrome 52.0.2743.116 m
string.test x 6,262,369 ops/sec ±1.33% (64 runs sampled)
string.match x 7,424,720 ops/sec ±1.24% (64 runs sampled)
compare x 61,916,203 ops/sec ±0.93% (65 runs sampled)
indexOf x 6,074,472 ops/sec ±0.20% (53 runs sampled)
slice x 21,237,136 ops/sec ±0.82% (65 runs sampled)
substring x 21,249,577 ops/sec ±0.97% (65 runs sampled)

Firefox 48.0.1
string.test x 2,950,793 ops/sec ±4.29% (35 runs sampled)
string.match x 3,744,966 ops/sec ±0.47% (40 runs sampled)
compare x 128,171,718 ops/sec ±1.23% (37 runs sampled)
indexOf x 13,164,769 ops/sec ±0.18% (36 runs sampled)
slice x 2,487,728,229 ops/sec ±0.20% (35 runs sampled)
substring x 611,629,405 ops/sec ±4.47% (30 runs sampled)

Internet Explorer 11.1000.14505.0
string.test x 3,243,133 ops/sec ±1.11% (60 runs sampled)
string.match x 2,888,071 ops/sec ±1.34% (59 runs sampled)
compare x 7,986,453 ops/sec ±0.43% (61 runs sampled)
indexOf x 2,883,355 ops/sec ±0.27% (62 runs sampled)
slice x 3,113,989 ops/sec ±1.38% (53 runs sampled)
substring x 3,195,738 ops/sec ±2.23% (59 runs sampled)

Edge 39.14905.1000.0
string.test x 3,236,581 ops/sec ±1.43% (59 runs sampled)
string.match x 2,516,817 ops/sec ±1.15% (59 runs sampled)
compare x 7,557,602 ops/sec ±0.26% (61 runs sampled)
indexOf x 2,630,144 ops/sec ±0.31% (62 runs sampled)
slice x 2,981,179 ops/sec ±1.14% (60 runs sampled)
substring x 2,901,881 ops/sec ±1.47% (61 runs sampled)

@developit
Copy link
Member

Firefox and whatever processor you have are very happy together lol

@developit
Copy link
Member

developit commented Aug 19, 2016

Just had a thought: if we wrap the test strings in functions and invoke them on each run with a counter appended, that will disable any crazy caching stuff that might be in play. Here's an updated fiddle to check on Firefox: https://jsfiddle.net/developit/84q4ema6/9/

It's still showing as compare for me, but by a significantly reduced margin:

I would really love it if there was a way to inspect the bytecode Firefox is generating for .slice() - maybe it's some sort of super optimized case? In Chrome it's just a normal CallWithDescriptor.

@c2h5oh
Copy link
Contributor Author

c2h5oh commented Aug 19, 2016

With this I have compare, indexOf, slice and substring within 10% of each other (with compare taking lead) in Chrome
In FF it's slice/substring about 40% faster than compare.

A little saner

@developit developit added this to Code-Complete in 8.0 Mar 14, 2017
@developit developit moved this from Code-Complete to Merged in 8.0 Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
8.0
Merged
Development

Successfully merging this pull request may close these issues.

None yet

2 participants