Skip to content
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

module slow, speed it up #22

Merged
merged 4 commits into from
Apr 26, 2013
Merged

module slow, speed it up #22

merged 4 commits into from
Apr 26, 2013

Conversation

hnry
Copy link
Contributor

@hnry hnry commented Apr 22, 2013

This module is slow.

Using stubs, and not actually doing any real session loading, purely just testing the module it is more than 12 times slower compared to not using this module.

I feel like this module can be faster, 12 times slower is too much.

My results look something like this out of a run count of 10000000:

Without using this module (aka the baseline)

  • op/s: 10004360 | time: 0.99956419

With this module (but no real session loading, it's all stubbed) so this is purely testing the module's performance with no outside factors

  • op/s: 780283 | time: 12.815870532

@hnry hnry mentioned this pull request Apr 22, 2013
@hnry
Copy link
Contributor Author

hnry commented Apr 22, 2013

After applying hnry@24922ea, the benchmark results look like this:

  • op/s: 1284548 | time: 7.784841399

@hnry
Copy link
Contributor Author

hnry commented Apr 22, 2013

hnry@d9c18d9
( last commit )
Very minor improvement but it does seem to consistently give a few thousand op/s more

  • op/s: 1313087 | time: 7.615640343

@wcamarao
Copy link
Owner

@LoveBear, I really appreciate your contribution, however I ran your benchmark with and without your changes and didn't notice a performance improvement, so I won't merge this change at least for now.

@hnry
Copy link
Contributor Author

hnry commented Apr 26, 2013

Are you sure you ran the benchmark correctly?

Without your module just the baseline:

node bench/bench

With your module:

node bench/bench session

If you did run it correctly, it would be very surprising to me considering

  1. bind is slow
  2. you don't even need bind or call, or fancy scoping

wcamarao pushed a commit that referenced this pull request Apr 26, 2013
module slow, speed it up
@wcamarao wcamarao merged commit 8f55f0c into wcamarao:master Apr 26, 2013
@wcamarao
Copy link
Owner

You're right, I had missed the argv on bench.js:39 so I haven't called with a 'session' arg. Now I see:

  • op/s: 2111041 | time: 4.736998569 (original)
  • op/s: 3659895 | time: 2.732318501 (modified)

Thanks for looking into this.

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.

None yet

2 participants