XOR Check for strings

[mcdonnelldean]

CC: @mcollina @davidmarkclements

In a weird twist of fate, on average, the XOR check is faster than simple equality.

With XOR

threeEntries*100000: 268ms
fiveHundredEntries*100000: 2396ms
fiveHundredEntriesAndProperties*100000: 2418ms
fiveHundredEntriesAndKnownProperties*100000: 2440ms
patrunThreeEntries*100000: 4095ms
patrunFiveHundredEntriesAndProperties*100000: 18429ms
threeEntries*100000: 266ms
fiveHundredEntries*100000: 2418ms
fiveHundredEntriesAndProperties*100000: 2400ms
fiveHundredEntriesAndKnownProperties*100000: 2441ms
patrunThreeEntries*100000: 4106ms
patrunFiveHundredEntriesAndProperties*100000: 18484ms

Simple === check

threeEntries*100000: 272ms
fiveHundredEntries*100000: 2435ms
fiveHundredEntriesAndProperties*100000: 2408ms
fiveHundredEntriesAndKnownProperties*100000: 2416ms
patrunThreeEntries*100000: 4132ms
patrunFiveHundredEntriesAndProperties*100000: 18291ms
threeEntries*100000: 268ms
fiveHundredEntries*100000: 2415ms
fiveHundredEntriesAndProperties*100000: 2414ms
fiveHundredEntriesAndKnownProperties*100000: 2582ms
patrunThreeEntries*100000: 4239ms
patrunFiveHundredEntriesAndProperties*100000: 19192ms

Having said that the variance is so little over more than 10 runs that they are for all intents and purposes comparable performance wise. Because of this I have opted to make it the default rather than providing an option to enable.

Thoughts?

[coveralls]

Coverage decreased (-0.2%) to 98.465% when pulling 24d02a6fbd54e176e32859fcb13c4d992ab8619d on xor into 56ed0aaef42ec4c6aeee9588ce14878e695154f2 on master.

[mcdonnelldean]

@mcollina Re coveralls, I can improve coverage but it will involve moving the equal() function and exposing it. Thoughts?

[coveralls]

Coverage increased (+0.3%) to 98.972% when pulling b81eda97dec2d555996ca5a64fa06520cacd31e3 on xor into 56ed0aaef42ec4c6aeee9588ce14878e695154f2 on master.

[coveralls]

Coverage increased (+0.5%) to 99.229% when pulling 392b0e78971e10b1083c574c1d7584a619e25b71 on xor into 56ed0aaef42ec4c6aeee9588ce14878e695154f2 on master.

[coveralls]

Coverage increased (+0.03%) to 98.728% when pulling 0f360418d378da766142db2ab46d6e088e0ca8b2 on xor into 56ed0aaef42ec4c6aeee9588ce14878e695154f2 on master.

[coveralls]

Coverage decreased (-0.2%) to 98.481% when pulling 41ecc2c0f28a291046622342fe888c60e13d0ae3 on xor into 56ed0aaef42ec4c6aeee9588ce14878e695154f2 on master.

[mcdonnelldean]

@mcollina You can set a number in coveralls so that coverage doesn't report errors like above. Right now I think it will fire every time there is any decrease.

@davidmarkclements @mcollina Could you sanity check something. Since we return early, are we defeating the purpose of the XOR check in the first place since it's for time based comparisons.

[mcollina]

You can set a number in coveralls so that coverage doesn't report errors like above. Right now I think it will fire every time there is any decrease.

@mcdonnelldean I hope it's good now.

Could you sanity check something. Since we return early, are we defeating the purpose of the XOR check in the first place since it's for time based comparisons.

Nope, because you can have a lot of patterns like { cmd: 'x', token: VARIABLE }.

[mcollina]

LGTM for me.

[mcdonnelldean]

Awesome, and your fix worked, it now passes fully. I'm stuck for time for release until later this afternoon.