Performance improvement.

[manubb]

Avoid creation of possibly large object to check if an object is actually empty.

[nkzawa]

👍

[defunctzombie]

What kind of perf improvement did this yield? Would be nice to have a benchmark script that can prove this is better.

[nkzawa]

I thought this would be obviously faster, but it's not :(

http://jsperf.com/for-in-or-keys-length (benchmark on browsers, just for reference)

[manubb]

On Chrome, isEmpty is faster than !Object.keys(data).length (it is not the case with Firefox). I suggest to have a look at:
http://jsperf.com/for-in-or-keys-length/2
on Chrome and run the following script with node.js:

var nIter = 1000;
var data = [];
for (var i = 0; i < 50000; i++) {
data.push(1);
}

function isEmpty(data) {
for (var k in data) {
if (data.hasOwnProperty(k)) {
return false;
}
}
return true;
}

console.time("isEmpty");
for (var k = 0; k < nIter; k++) {
isEmpty(data);
}
console.timeEnd("isEmpty");

console.time("!Object.keys(data).length");
for (var k = 0; k < nIter; k++) {
!Object.keys(data).length;
}
console.timeEnd("!Object.keys(data).length");

[nkzawa]

@manubb sorry, my setup for benchmark was wrong. data should be an object instead of an array.

http://jsperf.com/for-in-or-keys-length/3

[manubb]

On http://jsperf.com/for-in-or-keys-length/3 on Chrome, isEmpty is almost 4 times faster.

[kapouer]

The performance on firefox was bugging me... the test is wrong ! You're using integers as keys,
leaving room for array optimizations (i won't bet firefox is doing that, though).
Fixed there http://jsperf.com/for-in-or-keys-length/5

[kapouer]

And a fixed test for nodejs

var nIter = 1000;
var data = {};
for (var i = 0; i < 100000; i++) {
        data["key" + i] = "val" + i;
}

function isEmpty(data) {
        for (var k in data) {
                if (data.hasOwnProperty(k)) {
                        return false;
                }
        }
        return true;
}

console.time("isEmpty");
for (var k = 0; k < nIter; k++) {
        isEmpty(data);
}
console.timeEnd("isEmpty");

console.time("!Object.keys(data).length");
for (var k = 0; k < nIter; k++) {
        !Object.keys(data).length;
}
console.timeEnd("!Object.keys(data).length");

i get on nodejs 0.10 (v8 3.14)

isEmpty: 5609ms
!Object.keys(data).length: 6432ms

which is an improvement, but not that radical compared to the previous figures.

[kapouer]

Keeping track of a "size" property is the only proper way to avoid that O(n) lookup.
That would mean expose an interface for adding and removing sockets from rooms instead
of giving direct access to it. I believe it would make socket.io much less of a cpu eater on
cases where a lot of clients come and go.
Having an interface will also let the internals switch to an ES6 Map in the future.

[manubb]

I have posted a new pull request #30 so that deciding emptyness for a room is in O(1). Hence i close this pull request.