bcrypt.js
bcrypt.js copied to clipboard
Change of method of concatination
Looking thru the code, I can see (naturally) multiple places with '.push' Especially in Chrome and Edge browsers there can be gained a lot if used another method.
Try to have a look at this test: https://jsperf.com/join-concat/2
I ran this test with (Chrome, IE, Edge and Firefox) on Windows 10 and here you can see that big differences: https://github.com/ricmoo/aes-js/issues/33#issuecomment-332121243
There are several places, most likely, that could use a fixed-size array instead. That should be even better than string concatenation. Hasn't been a priority so far.
I'm not that convinced that fixed array is faster, but maybe that could just be my lack of skills:
for (var res = new Array(10000), i = 0; i < 10000; i++) {
res[i] = 'a';
}
res = res.join("");
Better, but still way slower than 'pure string concatenation':
It is the last joining that kills the usage of array:
res.concat('a')
is fastest. https://jsbench.me/d8j89wmlki/1
Indeed:
I ran the above tests 3 times and all showed the above.
IMHO the conclusion is that if the end result is a string use .concat, if the end result is an array, use the 'fixed array' with the size initially set.
res.concat('a')
does not mutates original string. It returns new one (just like concat for arrays). So it should be res = res.concat('a')
.
@vonagam yeah you're right, my bad. It actually seems to be the slowest.
Just ran a new benchmark:
When using res.join it is absolutely the slowest of them all and since we are working with strings then we have to use res.join.
Looks like += and .concat are the same. As I see it we should just switch to +=. That is simple and efficient. @dcodeIO What do you say? Will you review and accept a PR where += is used?