search

jshttp / fresh

HTTP request freshness testing
MIT License
161 stars 28 forks source link

Minor perf #22

Closed billouboq closed 7 years ago

billouboq commented 7 years ago

Minor perf improvement

dougwilson commented 7 years ago

Nice @billouboq ! I'm wondering about the following, if you can help me out:

1) Would you mind sharing the benchmark code you used? Sometimes these types of changes can speed up one version of Node.js but hurt others, so we want to make sure the change is neutral to positive on all versions of Node.js this module supports.

billouboq commented 7 years ago

Would you mind sharing the benchmark code you used? Sometimes these types of changes can speed up one version of Node.js but hurt others, so we want to make sure the change is neutral to positive on all versions of Node.js this module supports.

The regex part was tested on chrome 56 in jsperf.

And for the every replacement I think for loop is faster than any array functions for now in current v8 versions. Also every check for all value in the array while the for loop stops when the condition is true so less work in some cases.

Do you want me to benchmark all this ?

dougwilson commented 7 years ago

Oops, I merged & released this, but the for loop behaves differently and caused a regression :S I'm fixing it plus adding a test for the future :)