-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Improved performance of Pipeline.exec #991
Conversation
if (!buffers) { | ||
buffers = []; | ||
} | ||
if (typeof data === "string") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data is ignored if its already a buffer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data
cannot be a Buffer. I cleaned up the code a bit to make this clear.
Your comment made me aware that buffers
is actually never re-set, so I did that, too.
LGTM, @luin if you are ok with the PR I'd be happy to merge :) |
I'll merge this in, I believe releases are going through /next tag and it should be safe |
## [4.14.4](v4.14.3...v4.14.4) (2019-11-22) ### Bug Fixes * improved performance of Pipeline.exec ([#991](#991)) ([86470a8](86470a8))
🎉 This PR is included in version 4.14.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Thank you for merging this! I totally missed this pull request. The code LGTM. |
## [4.14.4](redis/ioredis@v4.14.3...v4.14.4) (2019-11-22) ### Bug Fixes * improved performance of Pipeline.exec ([#991](redis/ioredis#991)) ([86470a8](redis/ioredis@86470a8))
Execution cost of Pipeline.exec can be reduced to 50% or lower. This is
accomplished by first filling up an Array of Buffers and then calling
Buffer.concat
instead of calling
Buffer.concat
for each command.In our use-case (~3000 commands in a single Pipeline) wall-clock-runtime went
down from 1.3sec to 500ms.