Skip to content

Use a Room class to efficiently track room size.#30

Merged
rauchg merged 8 commits intosocketio:masterfrom
manubb:master-patched
Dec 3, 2015
Merged

Use a Room class to efficiently track room size.#30
rauchg merged 8 commits intosocketio:masterfrom
manubb:master-patched

Conversation

@manubb
Copy link

@manubb manubb commented Jun 3, 2015

Performance improvement in room management. This follows pull request #29.

@manubb manubb mentioned this pull request Jun 3, 2015
@kapouer
Copy link
Contributor

kapouer commented Jun 10, 2015

Hello @rauchg, do you think this has a chance to get accepted ?

@nkzawa
Copy link
Contributor

nkzawa commented Nov 22, 2015

Thanks! Do you have any benchmark results ?
btw maybe Room class implementation can be replaced by Map in the future since it has size property.

@kapouer
Copy link
Contributor

kapouer commented Nov 22, 2015

I don't. This PR should be rewritten cleanly along with benchmarks, indeed.

@rauchg
Copy link
Contributor

rauchg commented Nov 22, 2015

I'm pretty sure the new way will be faster? Object.keys().length should be costly.

@kapouer
Copy link
Contributor

kapouer commented Nov 22, 2015

I'm in favor of using Map too. This PR was written when using nodejs 0.10...

rauchg added a commit that referenced this pull request Dec 3, 2015
Use a Room class to efficiently track room size.
@rauchg rauchg merged commit d16e46d into socketio:master Dec 3, 2015
@rauchg
Copy link
Contributor

rauchg commented Dec 3, 2015

This looks really great @kapouer. Merged. @darrachequesne we are still planning on ES6 Map

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants