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: improve allocation and from(arrayLike) performance #10443

Closed
wants to merge 2 commits into from

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Dec 25, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)
  • buffer
Description of change

Improve Buffer allocation performance (up to ~11%) by making assertSize() inlineable and improve Buffer.from(arrayLike) performance (~50%) by replacing the usage of the in operator when checking for a length property.

Relevant benchmark results:

Buffer allocation:

                                                                   improvement significant      p.value
buffers/buffer-creation.js n=1024 len=10 type="buffer()"                3.67 %         *** 1.907380e-31
buffers/buffer-creation.js n=1024 len=10 type="fast-alloc-fill"         4.34 %         *** 3.497544e-11
buffers/buffer-creation.js n=1024 len=10 type="fast-alloc"              9.94 %         *** 7.596522e-44
buffers/buffer-creation.js n=1024 len=10 type="fast-allocUnsafe"        5.05 %         *** 2.696345e-51
buffers/buffer-creation.js n=1024 len=10 type="slow-allocUnsafe"        3.49 %         *** 1.076653e-07
buffers/buffer-creation.js n=1024 len=10 type="slow"                   11.22 %         *** 5.188153e-61
buffers/buffer-creation.js n=1024 len=1024 type="buffer()"              1.42 %          ** 1.348395e-03
buffers/buffer-creation.js n=1024 len=1024 type="fast-alloc-fill"       2.52 %         *** 1.446582e-06
buffers/buffer-creation.js n=1024 len=1024 type="fast-alloc"            5.38 %         *** 1.283277e-27
buffers/buffer-creation.js n=1024 len=1024 type="fast-allocUnsafe"      2.18 %         *** 2.896226e-06
buffers/buffer-creation.js n=1024 len=1024 type="slow-allocUnsafe"      3.41 %         *** 6.929603e-10
buffers/buffer-creation.js n=1024 len=1024 type="slow"                  9.38 %         *** 1.607505e-56
buffers/buffer-creation.js n=1024 len=2048 type="buffer()"              2.13 %         *** 2.849077e-05
buffers/buffer-creation.js n=1024 len=2048 type="fast-alloc-fill"       2.39 %         *** 1.291052e-07
buffers/buffer-creation.js n=1024 len=2048 type="fast-alloc"            3.96 %         *** 2.701098e-22
buffers/buffer-creation.js n=1024 len=2048 type="fast-allocUnsafe"      1.57 %         *** 5.975363e-04
buffers/buffer-creation.js n=1024 len=2048 type="slow-allocUnsafe"      4.54 %         *** 3.838296e-15
buffers/buffer-creation.js n=1024 len=2048 type="slow"                  8.52 %         *** 6.733714e-46
buffers/buffer-creation.js n=1024 len=4096 type="buffer()"              3.62 %         *** 1.399411e-10
buffers/buffer-creation.js n=1024 len=4096 type="fast-alloc-fill"       1.85 %         *** 2.706257e-05
buffers/buffer-creation.js n=1024 len=4096 type="fast-alloc"            1.30 %         *** 1.625928e-04
buffers/buffer-creation.js n=1024 len=4096 type="fast-allocUnsafe"      4.14 %         *** 4.122709e-15
buffers/buffer-creation.js n=1024 len=4096 type="slow-allocUnsafe"      3.78 %         *** 5.601502e-13
buffers/buffer-creation.js n=1024 len=4096 type="slow"                  8.09 %         *** 6.466906e-46
buffers/buffer-creation.js n=1024 len=8192 type="buffer()"              2.54 %         *** 1.426874e-05
buffers/buffer-creation.js n=1024 len=8192 type="fast-alloc-fill"       1.16 %          ** 1.392665e-03
buffers/buffer-creation.js n=1024 len=8192 type="fast-alloc"            0.83 %          ** 2.391328e-03
buffers/buffer-creation.js n=1024 len=8192 type="fast-allocUnsafe"      5.33 %         *** 1.811794e-19
buffers/buffer-creation.js n=1024 len=8192 type="slow-allocUnsafe"      4.85 %         *** 1.764478e-16
buffers/buffer-creation.js n=1024 len=8192 type="slow"                  8.18 %         *** 1.006915e-42

Buffer.from(arrayLike):

                                                       improvement significant      p.value
buffers/buffer-from.js n=10024 len=10 source="object"       48.85 %         *** 4.055606e-41
buffers/buffer-from.js n=10024 len=2048 source="object"     49.44 %         *** 2.809991e-51

/cc @nodejs/buffer
CI: https://ci.nodejs.org/job/node-test-pull-request/5580/

@mscdex mscdex added the buffer Issues and PRs related to the buffer subsystem. label Dec 25, 2016
@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. dont-land-on-v7.x labels Dec 25, 2016
@mscdex mscdex added the performance Issues and PRs related to the performance of Node.js. label Dec 25, 2016
@mscdex mscdex changed the title Buffer allocate perf buffer: improve allocation and from(arrayLike) performance Dec 25, 2016
@@ -38,6 +38,9 @@ function createUnsafeBuffer(size) {
return new FastBuffer(createUnsafeArrayBuffer(size));
}

/*function newArrayBuffer(size) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Removed.

Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM with a question.

assertSize() is adjusted to be inlineable according to V8's default
function size limits when determining inlineability. This results in
up to 11% performance gains when allocating any kind of Buffer.
This change results in ~50% improvement when creating a Buffer from
an array-like object.
@Trott
Copy link
Member

Trott commented Dec 25, 2016

Probably too tangentially related to this PR to be worth doing, but just in case: There's a != on line 172 of lib/buffer.js that seems to be calling out to be replaced with !==.

Copy link
Member

@ChALkeR ChALkeR left a comment

Choose a reason for hiding this comment

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

LGTM after CI.

@jasnell
Copy link
Member

jasnell commented Dec 27, 2016

jasnell pushed a commit that referenced this pull request Dec 27, 2016
assertSize() is adjusted to be inlineable according to V8's default
function size limits when determining inlineability. This results in
up to 11% performance gains when allocating any kind of Buffer.

Avoid avoids use of in, resulting in ~50% improvement when creating
a Buffer from an array-like object.

PR-URL: #10443
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@jasnell
Copy link
Member

jasnell commented Dec 27, 2016

Landed in 13a4887

@jasnell jasnell closed this Dec 27, 2016
@mscdex mscdex deleted the buffer-allocate-perf branch December 29, 2016 02:20
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
assertSize() is adjusted to be inlineable according to V8's default
function size limits when determining inlineability. This results in
up to 11% performance gains when allocating any kind of Buffer.

Avoid avoids use of in, resulting in ~50% improvement when creating
a Buffer from an array-like object.

PR-URL: #10443
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
evanlucas added a commit that referenced this pull request Jan 3, 2017
Notable changes:

* buffer:
  - Improve performance of Buffer allocation by ~11% (Brian White) #10443
  - Improve performance of Buffer.from() by ~50% (Brian White) #10443
* events: Improve performance of EventEmitter.once() by ~27% (Brian White) #10445
* http: Improve performance of http server by ~7% (Brian White) #6533
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
assertSize() is adjusted to be inlineable according to V8's default
function size limits when determining inlineability. This results in
up to 11% performance gains when allocating any kind of Buffer.

Avoid avoids use of in, resulting in ~50% improvement when creating
a Buffer from an array-like object.

PR-URL: #10443
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
evanlucas added a commit that referenced this pull request Jan 4, 2017
Notable changes:

* buffer:
  - Improve performance of Buffer allocation by ~11% (Brian White) #10443
  - Improve performance of Buffer.from() by ~50% (Brian White) #10443
* events: Improve performance of EventEmitter.once() by ~27% (Brian White) #10445
* fs: Allow passing Uint8Array to fs methods where Buffers are supported. (Anna Henningsen) #10382
* http: Improve performance of http server by ~7% (Brian White) #6533
* npm: Upgrade to v4.0.5 (Kat Marchán) #10330

PR-URL: #10589
evanlucas added a commit that referenced this pull request Jan 4, 2017
Notable changes:

* buffer:
  - Improve performance of Buffer allocation by ~11% (Brian White) #10443
  - Improve performance of Buffer.from() by ~50% (Brian White) #10443
* events: Improve performance of EventEmitter.once() by ~27% (Brian White) #10445
* fs: Allow passing Uint8Array to fs methods where Buffers are supported. (Anna Henningsen) #10382
* http: Improve performance of http server by ~7% (Brian White) #6533
* npm: Upgrade to v4.0.5 (Kat Marchán) #10330

PR-URL: #10589
@MylesBorins
Copy link
Contributor

Adding LTS watch, likely should bake a bit longer before landing.

imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
    Notable changes:

    * buffer:
      - Improve performance of Buffer allocation by ~11% (Brian White) nodejs/node#10443
      - Improve performance of Buffer.from() by ~50% (Brian White) nodejs/node#10443
    * events: Improve performance of EventEmitter.once() by ~27% (Brian White) nodejs/node#10445
    * fs: Allow passing Uint8Array to fs methods where Buffers are supported. (Anna Henningsen) nodejs/node#10382
    * http: Improve performance of http server by ~7% (Brian White) nodejs/node#6533
    * npm: Upgrade to v4.0.5 (Kat Marchán) nodejs/node#10330

    PR-URL: nodejs/node#10589

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
@MylesBorins MylesBorins added baking-for-lts PRs that need to wait before landing in a LTS release. and removed lts-watch-v4.x labels May 8, 2017
@MylesBorins MylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018
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.

9 participants