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

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

Closed
wants to merge 1 commit into from

Conversation

JacksonTian
Copy link
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 mscdex added the buffer Issues and PRs related to the buffer subsystem. label Dec 17, 2015
@mscdex
Copy link
Contributor

mscdex commented Dec 17, 2015

@bnoordhuis
Copy link
Member

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

@@ -0,0 +1,18 @@
'use strict';

var common = require('../common.js');
Copy link
Member

Choose a reason for hiding this comment

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

const please

@jasnell
Copy link
Member

jasnell commented Dec 17, 2015

LGTM with minor nit

@cjihrig
Copy link
Contributor

cjihrig commented Dec 17, 2015

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

@JacksonTian
Copy link
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
Copy link
Member

jasnell commented Dec 24, 2015

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
Copy link
Member

jasnell commented Dec 24, 2015

Landed in 5396baf

@jasnell jasnell closed this Dec 24, 2015
@JacksonTian 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 MylesBorins mentioned this pull request Jan 19, 2016
@ChALkeR 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>
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