Increase performance of generateId by avoiding sync calls to crypto.randomBytes #1008

Merged
merged 3 commits into from Sep 10, 2012

Conversation

Projects
None yet
3 participants
@faeldt

faeldt commented Aug 28, 2012

As mentioned in LearnBoost#934, generateId contains a sync call to crypto.randomBytes. To try to minimize the impact of this, I've added a new function called getRandomBytes that gets bytes in chunks using async calls and keeps them in a buffer to deliver when needed, graciously falling back to the sync call only when the buffer is temporarily empty. This result in a significant decrease in blocking calls compared to the current implementation.

I tried making 100k calls to this.getRandomBytes(16) (one per process.nextTick()), and out of these only about 3k fell through to the sync crypto.randomBytes(), while the rest were delivered from the buffer.

The average return time of this.getRandomBytes(16) is roughly 0.00100, compared to a crypto.randomBytes(16) call which clocks in at an average of about 0.00600.

@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 28, 2012

This pull request fails (merged da91c89 into 21a3733).

This pull request fails (merged da91c89 into 21a3733).

@rauchg

This comment has been minimized.

Show comment Hide comment
@rauchg

rauchg Aug 28, 2012

Contributor

@faeldt it'd be really cool if you could make the generation a simple node module, to avoid all this code?

Contributor

rauchg commented Aug 28, 2012

@faeldt it'd be really cool if you could make the generation a simple node module, to avoid all this code?

@faeldt

This comment has been minimized.

Show comment Hide comment
@faeldt

faeldt Aug 29, 2012

@guille no problem. wasn't sure if you'd be okay with introducing another dependency.

faeldt commented Aug 29, 2012

@guille no problem. wasn't sure if you'd be okay with introducing another dependency.

@rauchg

This comment has been minimized.

Show comment Hide comment
@rauchg

rauchg Aug 29, 2012

Contributor

Dependencies and smaller, modular codebases are all I'm for.

Contributor

rauchg commented Aug 29, 2012

Dependencies and smaller, modular codebases are all I'm for.

@faeldt

This comment has been minimized.

Show comment Hide comment
@faeldt

faeldt Sep 10, 2012

@guille: Moved both my new randomBytes and the entire generateId function. As clean as it gets.

faeldt commented Sep 10, 2012

@guille: Moved both my new randomBytes and the entire generateId function. As clean as it gets.

rauchg added a commit that referenced this pull request Sep 10, 2012

Merge pull request #1008 from faeldt/master
Increase performance of generateId by avoiding sync calls to crypto.randomBytes

@rauchg rauchg merged commit c3ba8a7 into socketio:master Sep 10, 2012

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment