FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

buffer: faster case for create Buffer from zero length buffer by JacksonTian · Pull Request #4326 · nodejs/node · GitHub

/ node Public

buffer: faster case for create Buffer from zero length buffer - #4326

Closed
JacksonTian wants to merge 1 commit into
nodejs:masterfrom
JacksonTian:empty_buffer2
Closed

buffer: faster case for create Buffer from zero length buffer#4326
JacksonTian wants to merge 1 commit into
nodejs:masterfrom
JacksonTian:empty_buffer2

Conversation

Copy link
Copy Markdown
Contributor

When create Buffer from a Buffer will copy data from old
to new even though length is zero.

This patch can improve edge case 4x faster.
following is benchmark results.

new: buffers/buffer_zero.js n=1024: 2463.53891
old: buffers/buffer_zero.js n=1024: 618.70801

mscdex added the buffer Issues and PRs related to the buffer subsystem. label Dec 17, 2015

mscdex commented Dec 17, 2015

Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Member

LGTM but the first line of the commit log is > 50 characters.

Comment thread benchmark/buffers/buffer_zero.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

const please

jasnell commented Dec 17, 2015

Copy link
Copy Markdown
Member

LGTM with minor nit

cjihrig commented Dec 17, 2015

Copy link
Copy Markdown
Contributor

LGTM if you swap out the vars for lets and consts.

Copy link
Copy Markdown
Contributor Author

Thanks @jasnell @cjihrig @bnoordhuis , Updated it by your suggestions.

When create Buffer from a Buffer will copy data
from old to new even though length is zero.

This patch can improve edge case 4x faster.
following is benchmark results.

new: buffers/buffer_zero.js n=1024: 2463.53891
old: buffers/buffer_zero.js n=1024: 618.70801

jasnell commented Dec 24, 2015

Copy link
Copy Markdown
Member

LGTM

jasnell pushed a commit that referenced this pull request Dec 24, 2015
When create Buffer from a Buffer will copy data
from old to new even though length is zero.

This patch can improve edge case 4x faster.
following is benchmark results.

new: buffers/buffer_zero.js n=1024: 2463.53891
old: buffers/buffer_zero.js n=1024: 618.70801

PR-URL: #4326
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell commented Dec 24, 2015

Copy link
Copy Markdown
Member

Landed in 5396baf

jasnell closed this Dec 24, 2015
JacksonTian deleted the empty_buffer2 branch December 24, 2015 02:28
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
When create Buffer from a Buffer will copy data
from old to new even though length is zero.

This patch can improve edge case 4x faster.
following is benchmark results.

new: buffers/buffer_zero.js n=1024: 2463.53891
old: buffers/buffer_zero.js n=1024: 618.70801

PR-URL: nodejs#4326
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 13, 2016
When create Buffer from a Buffer will copy data
from old to new even though length is zero.

This patch can improve edge case 4x faster.
following is benchmark results.

new: buffers/buffer_zero.js n=1024: 2463.53891
old: buffers/buffer_zero.js n=1024: 618.70801

PR-URL: #4326
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
When create Buffer from a Buffer will copy data
from old to new even though length is zero.

This patch can improve edge case 4x faster.
following is benchmark results.

new: buffers/buffer_zero.js n=1024: 2463.53891
old: buffers/buffer_zero.js n=1024: 618.70801

PR-URL: #4326
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins mentioned this pull request Jan 19, 2016
ChALkeR added the performance Issues and PRs related to the performance of Node.js. label Feb 16, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
When create Buffer from a Buffer will copy data
from old to new even though length is zero.

This patch can improve edge case 4x faster.
following is benchmark results.

new: buffers/buffer_zero.js n=1024: 2463.53891
old: buffers/buffer_zero.js n=1024: 618.70801

PR-URL: nodejs#4326
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buffer Issues and PRs related to the buffer subsystem. performance Issues and PRs related to the performance of Node.js.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL