Repository navigation
events.js spliceOne is slower than splice #14081
Description
- Version:8.0
- Platform:mac os
when I inspect the source code of event emitter, I find code and comments in events.js as below:
// About 1.5x faster than the two-arg version of Array#splice().
function spliceOne(list, index) {
for (var i = index, k = i + 1, n = list.length; k < n; i += 1, k += 1)
list[i] = list[k];
list.pop();
}But when test it, I find it's not faster but slower, the test code as below:
let mArr01 = [];
let mArr02 = [];
let length = 10000;
let randData;
for (let i = 0; i < length; i++) {
randData = Math.random();
mArr01.push(randData);
mArr02.push(randData);
}
console.time('Array splice');
for (let j = 0; j < 1000; j++) {
mArr01.splice(1000 - j, 1);
}
console.timeEnd('Array splice');
console.time('spliceOne splice');
for (let j = 0; j < 1000; j++) {
spliceOne(mArr02, 1000 - j);
}
console.timeEnd('spliceOne splice');And I get the result as below:
// first time
Array splice: 3.658ms
spliceOne splice: 18.979ms
// second time
Array splice: 4.846ms
spliceOne splice: 27.892ms
// third time
Array splice: 3.274ms
spliceOne splice: 21.974ms
so, Is there any reason for old version ? Or any other reasons?
Activity
Trott commentedon Jul 5, 2017
It's likely that the microbenchmark above is not applicable to the one-and-only situation in
events.jswherespliceOne()is used. (For starters,events.jsdoes not use it on an array of numbers. I'm not very benchmark-savvy, though. Others may be able to identify issues I don't see.)That said, there is a benchmark in our suite written specifically forevents.jsand it's not showing any significant difference betweenspliceOne()andlist.splice()one way or the other. It's entirely possible that thespliceOne()optimization was relevant when we were using an earlier version of V8 but that it no longer makes a difference.I've opened #14082 to remove the function.Trott commentedon Jul 5, 2017
By the way, the same function appears inurl.js. If you or anyone else wants to remove it from there and see how that affects the url benchmarks, please go for it. (It's late in my time zone and I'm going to sleep.)TimothyGu commentedon Jul 5, 2017
@Sunqinying Your benchmark shows the danger of using microbenchmarks to predict performance of a larger system. As I have observed in #14082 (comment), within certain contexts,
spliceOneis still much faster than.splice(pos, 1). For more information, @bmeurer's blog post is one of the recent seminal works on this topic.Sunqinying commentedon Jul 5, 2017
@Trott @TimothyGu Thanks for you reply , I'm not really understand how node develop, I will try to understand.
Sunqinying commentedon Jul 5, 2017
@Trott @TimothyGu Should I close the issue ?
Trott commentedon Jul 5, 2017
I'd say yes, close it. You can always re-open it if you have additional questions. (The #node-dev channel on Freenode IRC might be a good place for questions about Node.js internals and development process.)
Sunqinying commentedon Jul 5, 2017
@Trott Thanks! I will closed it.