-
Notifications
You must be signed in to change notification settings - Fork 68
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
Refactor Variable access #57
Comments
Hey Thanks for this, it's true that this part need to be refactored! I try to write while I learn, so I'm going updating when I understand better about JS internal Related with this part, also included something I read from https://hackernoon.com/how-to-make-the-fastest-promise-library-f632fd69f3cb could be interesting about avoid creating unnecessary variables, functions and instances. PR are welcome 😄 |
I'd love to make you a PR but unfortunately I speak better JavaScript than english 😂 |
I also would like to mention that on this part I tried to benchmark the last tip (while loop with decrementing index) and it's super slow on Chrome (V8). It's like 10 times slower than a "standard for loop". Can you confirm that or am I doing something wrong ? |
@PierreCapo Interesting, I could only make some assumptions here without the code and deeper analysis. Another optimization that engines do is called loop unrolling. It might be Chrome not unrolling this loop because of the decrement? Out of curiousity if you increment it instead do you have a difference? It's good to mention that it may also depend on how you initialized your array, its content and if it's sparsed. |
@ngryman please, do the PR! 🙏 @PierreCapo could be simple create a tiny benchmark and compare result. I did it with the following code 'use strict'
var bench = require('fastbench')
const n = 100000
const arr = Array.from({length: n}, (value, index) => index)
const EXPECTED_RESULT = 4999950000
var run = bench([
function forEach (done) {
let result = 0
arr.forEach(item => {
result = result + item
})
if (result !== EXPECTED_RESULT) throw Error('invalid result')
done()
},
function reduce (done) {
const result = arr.reduce((acc, item) => acc + item)
if (result !== EXPECTED_RESULT) throw Error('invalid result')
done()
},
function whileLoop (done) {
let result = 0
let index = arr.length
while (index--) result = result + arr[index]
if (result !== EXPECTED_RESULT) throw Error('invalid result')
done()
}
], 1000)
// run them two times
run(run) results on my local (node 9.x) ❯ node index.js
forEach*1000: 1779.441ms
reduce*1000: 2863.238ms
whileLoop*1000: 253.822ms
forEach*1000: 2116.119ms
reduce*1000: 1727.525ms
whileLoop*1000: 162.896ms |
In addition, I have open another issue related to be possible live demo on the site (like this benchmark) #70 |
Hey, very nice initiative and website 👏
The
items.length
part in Variable access is not always true and even probably wrong for your example. Most modern VMs apply loop-invariant code motion.Array#length
is no exception, there is a very detailed article that explains what happens here: http://mrale.ph/blog/2014/12/24/array-length-caching.html.I'd suggest refactoring this part and mentioning that accessing
Array#length
IS OK (myth busted) if the loop body does not mutate the array or is simple enough.The text was updated successfully, but these errors were encountered: