src,buffer: allow heap-allocated typed arrays #26301

addaleax · 2019-02-25T18:52:58Z

I noticed that the reason we’ve been disabling this has not actually been true, so it seems to make sense to let the engine use its default values. /cc @nodejs/buffer @nodejs/v8

build: remove v8_typed_array_max_size_in_heap option

This was added in 16f86d6, based on
the assumption that otherwise, the memory behind ArrayBuffer
instances could be moved around on the heap while native code
holds references to it.

This does not match what V8 actually does (and also did at the time):

The option/build variable was about always only about TypedArrays,
not ArrayBuffers. Calls like new ArrayBuffer(4) call into C++
regardless of the option value, but calls like new Uint8Array(4)
would not call into C++ under V8 defaults.
When first accessing a heap-allocated TypedArray’s ArrayBuffer,
whether that is through the JS .buffer getter or the C++
ArrayBufferView::Buffer() function, a copy of the contents is
created using the ArrayBuffer allocator and stored as the
(permanent, unmovable) backing store.

As a consequence, the memory returned by ArrayBuffer::GetContents()
is not moved around, because it is fixed once the ArrayBuffer
object itself first comes into explicit existence in any way.

Removing this build option significantly speeds up creation of typed
arrays from JS:

$ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter buffer-creation.js buffers | Rscript benchmark/compare.R
                                                                     confidence improvement accuracy (*)     (**)    (***)
 buffers/buffer-creation.js n=1024 len=10 type='buffer()'                  ***    593.66 %      ±28.64%  ±41.10%  ±60.36%
 buffers/buffer-creation.js n=1024 len=10 type='fast-alloc-fill'           ***    675.42 %      ±90.67% ±130.24% ±191.54%
 buffers/buffer-creation.js n=1024 len=10 type='fast-alloc'                ***    663.55 %      ±58.41%  ±83.87% ±123.29%
 [... see commit log for more, statistically insignificant results ...]

The performance gains effect are undone once native code accesses
the underlying ArrayBuffer, but then again that a) does not happen for
all TypedArrays, and b) it should also make sense to look into using
ArrayBufferView::CopyContents() in some places, which is made
specifically to avoid such a performance impact and allows us to
use the benefits of heap-allocated typed arrays.

Refs: 16f86d6
Refs: #2893
Refs: 74178a5#commitcomment-13250880
Refs: http://logs.libuv.org/node-dev/2015-09-15

src: allow not materializing ArrayBuffers from C++

Where appropriate, use a helper that wraps around
ArrayBufferView::Buffer() or ArrayBufferView::CopyContents()
rather than Buffer::Data(), as that may help to avoid materializing
the underlying ArrayBuffer when reading small typed arrays from C++.
This allows keeping the performance benefits of the faster creation of
heap-allocated small typed arrays in many cases.

buffer: avoid materializing ArrayBuffer for creation

Do not create an ArrayBuffer if the engine’s settings avoid it
and we don’t need it.

test: verify heap buffer allocations occur

Check that small typed arrays, including Buffers (unless allocated
by Buffer.allocUnsafe()), are indeed heap-allocated.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

nodejs-github-bot · 2019-02-25T18:53:01Z

@addaleax sadly an error occured when I tried to trigger a build :(

This was added in 16f86d6, based on the assumption that otherwise, the memory behind `ArrayBuffer` instances could be moved around on the heap while native code holds references to it. This does not match what V8 actually does (and also did at the time): - The option/build variable was about always only about TypedArrays, not ArrayBuffers. Calls like `new ArrayBuffer(4)` call into C++ regardless of the option value, but calls like `new Uint8Array(4)` would not call into C++ under V8 defaults. - When first accessing a heap-allocated TypedArray’s `ArrayBuffer`, whether that is through the JS `.buffer` getter or the C++ `ArrayBufferView::Buffer()` function, a copy of the contents is created using the ArrayBuffer allocator and stored as the (permanent, unmovable) backing store. As a consequence, the memory returned by `ArrayBuffer::GetContents()` is not moved around, because it is fixed once the `ArrayBuffer` object itself first comes into explicit existence in any way. Removing this build option significantly speeds up creation of typed arrays from JS: $ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter buffer-creation.js buffers | Rscript benchmark/compare.R confidence improvement accuracy (*) (**) (***) buffers/buffer-creation.js n=1024 len=10 type='buffer()' *** 593.66 % ±28.64% ±41.10% ±60.36% buffers/buffer-creation.js n=1024 len=10 type='fast-alloc-fill' *** 675.42 % ±90.67% ±130.24% ±191.54% buffers/buffer-creation.js n=1024 len=10 type='fast-alloc' *** 663.55 % ±58.41% ±83.87% ±123.29% buffers/buffer-creation.js n=1024 len=10 type='fast-allocUnsafe' 3.10 % ±9.63% ±13.22% ±18.07% buffers/buffer-creation.js n=1024 len=10 type='slow-allocUnsafe' 4.67 % ±5.55% ±7.77% ±10.97% buffers/buffer-creation.js n=1024 len=10 type='slow' -2.48 % ±4.47% ±6.12% ±8.34% buffers/buffer-creation.js n=1024 len=1024 type='buffer()' -1.91 % ±4.71% ±6.45% ±8.79% buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc-fill' -1.34 % ±7.53% ±10.33% ±14.10% buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc' 0.52 % ±5.00% ±6.87% ±9.40% buffers/buffer-creation.js n=1024 len=1024 type='fast-allocUnsafe' 0.39 % ±5.65% ±7.78% ±10.67% buffers/buffer-creation.js n=1024 len=1024 type='slow-allocUnsafe' -0.13 % ±5.68% ±7.83% ±10.77% buffers/buffer-creation.js n=1024 len=1024 type='slow' -5.07 % ±7.15% ±9.80% ±13.35% buffers/buffer-creation.js n=1024 len=2048 type='buffer()' 0.57 % ±2.70% ±3.74% ±5.16% buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc-fill' -1.60 % ±4.96% ±6.79% ±9.25% buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc' 1.29 % ±3.79% ±5.20% ±7.09% buffers/buffer-creation.js n=1024 len=2048 type='fast-allocUnsafe' 2.73 % ±8.79% ±12.05% ±16.41% buffers/buffer-creation.js n=1024 len=2048 type='slow-allocUnsafe' -0.99 % ±6.27% ±8.65% ±11.91% buffers/buffer-creation.js n=1024 len=2048 type='slow' -5.98 % ±6.24% ±8.71% ±12.20% buffers/buffer-creation.js n=1024 len=4096 type='buffer()' -1.75 % ±3.48% ±4.78% ±6.56% buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc-fill' -3.18 % ±3.97% ±5.45% ±7.45% buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc' 2.05 % ±4.05% ±5.58% ±7.65% buffers/buffer-creation.js n=1024 len=4096 type='fast-allocUnsafe' 1.44 % ±5.51% ±7.63% ±10.57% buffers/buffer-creation.js n=1024 len=4096 type='slow-allocUnsafe' * -4.77 % ±4.30% ±5.90% ±8.06% buffers/buffer-creation.js n=1024 len=4096 type='slow' -3.31 % ±6.38% ±8.86% ±12.34% buffers/buffer-creation.js n=1024 len=8192 type='buffer()' 0.06 % ±2.70% ±3.77% ±5.31% buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc-fill' -1.20 % ±3.30% ±4.53% ±6.17% buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc' -1.46 % ±2.75% ±3.84% ±5.38% buffers/buffer-creation.js n=1024 len=8192 type='fast-allocUnsafe' 1.27 % ±4.69% ±6.49% ±8.98% buffers/buffer-creation.js n=1024 len=8192 type='slow-allocUnsafe' -1.68 % ±3.30% ±4.62% ±6.49% buffers/buffer-creation.js n=1024 len=8192 type='slow' -2.49 % ±3.24% ±4.44% ±6.07% (Re-running the outlier with 30 runs instead of 10:) buffers/buffer-creation.js n=1024 len=4096 type='slow-allocUnsafe' 2.06 % ±2.39% ±3.19% ±4.15% The performance gains effect are undone once native code accesses the underlying ArrayBuffer, but then again that a) does not happen for all TypedArrays, and b) it should also make sense to look into using `ArrayBufferView::CopyContents()` in some places, which is made specifically to avoid such a performance impact and allows us to use the benefits of heap-allocated typed arrays. Refs: nodejs@16f86d6 Refs: nodejs#2893 Refs: nodejs@74178a5#commitcomment-13250880 Refs: http://logs.libuv.org/node-dev/2015-09-15

Where appropriate, use a helper that wraps around `ArrayBufferView::Buffer()` or `ArrayBufferView::CopyContents()` rather than `Buffer::Data()`, as that may help to avoid materializing the underlying `ArrayBuffer` when reading small typed arrays from C++. This allows keeping the performance benefits of the faster creation of heap-allocated small typed arrays in many cases.

Do not create an `ArrayBuffer` if the engine’s settings avoid it and we don’t need it.

Check that small typed arrays, including `Buffer`s (unless allocated by `Buffer.allocUnsafe()`), are indeed heap-allocated.

psmarshall · 2019-02-26T10:03:56Z

I like it! There are probably a fair few libraries out there creating small typed arrays for things like vector arithmetic e.g. with Float64Array for 3D points or something, the idea of the split on/off-heap worlds is to keep these fast. Each typed array with an off-heap buffer also adds time to the GC as we have to explicitly finalize the off-heap backing store. Thanks for working on this 👍

mcollina

LGTM

I would like a CITGM run to avoid potential regressions on native, 3rd party modules.

addaleax · 2019-02-26T13:48:29Z

@mcollina Sure, will do, but to clarify: This is not an API change. It affects performance, not functionality.

mcollina · 2019-02-26T14:01:05Z

I agree. However it has been like this forever and some native addon could do something... interesting with it.

trevnorris · 2019-02-27T21:28:17Z

Here's an extract of my conversation with @indutny:

Sep 15 22:21:53 <indutny>	[17:59] trevnorris: I see now
Sep 15 22:21:53 <indutny>	[17:59] trevnorris: it is using fixed array for elements
Sep 15 22:21:53 <indutny>	[17:59] for small ArrayBuffers
Sep 15 22:21:53 <indutny>	[18:08] trevnorris: we need to allocate small buffers in old way
Sep 15 22:21:53 <indutny>	[18:20] trevnorris: yeah, it will move them
Sep 15 22:21:53 <indutny>	[18:20] trevnorris: we need to fix it

Comment from @jeisinger about this when the implementation first landed: #1451 (comment)

addaleax · 2019-02-28T09:20:26Z

@trevnorris Yeah, I read that – it sounded like @indutny thought that this option applies to ArrayBuffers, and @jeisinger’s comment was, err, not really going into whether the underlying memory is being moved?

Like, I totally understand the line of thought that was behind introducing this option, it’s just that it turns out we don’t actually need it.

jeisinger · 2019-02-28T09:30:10Z

I'm not sure which version of v8 you're using here, but the array buffer implementation changed quite a bit since I did that comment.

addaleax · 2019-02-28T21:35:55Z

CI: https://ci.nodejs.org/job/node-test-pull-request/21066/ (:white_check_mark:)
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1753/

addaleax · 2019-03-01T20:55:22Z

CITGM results are hard to read, but generally there’s nothing that isn’t in https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1752/ which is a directly preceding run for master.

Looking for results for native addons seems pointless, though, as Nan currently seems to be incompatible with master.

Given that I would still consider this a low-risk change, for addons or other code, and we run CITGM on releases anyway, I feel comfortable moving forward with this.

mcollina

LGTM

addaleax · 2019-03-01T21:15:59Z

Landed in b0b370c...adbe3b8

This was added in 16f86d6, based on the assumption that otherwise, the memory behind `ArrayBuffer` instances could be moved around on the heap while native code holds references to it. This does not match what V8 actually does (and also did at the time): - The option/build variable was about always only about TypedArrays, not ArrayBuffers. Calls like `new ArrayBuffer(4)` call into C++ regardless of the option value, but calls like `new Uint8Array(4)` would not call into C++ under V8 defaults. - When first accessing a heap-allocated TypedArray’s `ArrayBuffer`, whether that is through the JS `.buffer` getter or the C++ `ArrayBufferView::Buffer()` function, a copy of the contents is created using the ArrayBuffer allocator and stored as the (permanent, unmovable) backing store. As a consequence, the memory returned by `ArrayBuffer::GetContents()` is not moved around, because it is fixed once the `ArrayBuffer` object itself first comes into explicit existence in any way. Removing this build option significantly speeds up creation of typed arrays from JS: $ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter buffer-creation.js buffers | Rscript benchmark/compare.R confidence improvement accuracy (*) (**) (***) buffers/buffer-creation.js n=1024 len=10 type='buffer()' *** 593.66 % ±28.64% ±41.10% ±60.36% buffers/buffer-creation.js n=1024 len=10 type='fast-alloc-fill' *** 675.42 % ±90.67% ±130.24% ±191.54% buffers/buffer-creation.js n=1024 len=10 type='fast-alloc' *** 663.55 % ±58.41% ±83.87% ±123.29% buffers/buffer-creation.js n=1024 len=10 type='fast-allocUnsafe' 3.10 % ±9.63% ±13.22% ±18.07% buffers/buffer-creation.js n=1024 len=10 type='slow-allocUnsafe' 4.67 % ±5.55% ±7.77% ±10.97% buffers/buffer-creation.js n=1024 len=10 type='slow' -2.48 % ±4.47% ±6.12% ±8.34% buffers/buffer-creation.js n=1024 len=1024 type='buffer()' -1.91 % ±4.71% ±6.45% ±8.79% buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc-fill' -1.34 % ±7.53% ±10.33% ±14.10% buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc' 0.52 % ±5.00% ±6.87% ±9.40% buffers/buffer-creation.js n=1024 len=1024 type='fast-allocUnsafe' 0.39 % ±5.65% ±7.78% ±10.67% buffers/buffer-creation.js n=1024 len=1024 type='slow-allocUnsafe' -0.13 % ±5.68% ±7.83% ±10.77% buffers/buffer-creation.js n=1024 len=1024 type='slow' -5.07 % ±7.15% ±9.80% ±13.35% buffers/buffer-creation.js n=1024 len=2048 type='buffer()' 0.57 % ±2.70% ±3.74% ±5.16% buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc-fill' -1.60 % ±4.96% ±6.79% ±9.25% buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc' 1.29 % ±3.79% ±5.20% ±7.09% buffers/buffer-creation.js n=1024 len=2048 type='fast-allocUnsafe' 2.73 % ±8.79% ±12.05% ±16.41% buffers/buffer-creation.js n=1024 len=2048 type='slow-allocUnsafe' -0.99 % ±6.27% ±8.65% ±11.91% buffers/buffer-creation.js n=1024 len=2048 type='slow' -5.98 % ±6.24% ±8.71% ±12.20% buffers/buffer-creation.js n=1024 len=4096 type='buffer()' -1.75 % ±3.48% ±4.78% ±6.56% buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc-fill' -3.18 % ±3.97% ±5.45% ±7.45% buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc' 2.05 % ±4.05% ±5.58% ±7.65% buffers/buffer-creation.js n=1024 len=4096 type='fast-allocUnsafe' 1.44 % ±5.51% ±7.63% ±10.57% buffers/buffer-creation.js n=1024 len=4096 type='slow-allocUnsafe' * -4.77 % ±4.30% ±5.90% ±8.06% buffers/buffer-creation.js n=1024 len=4096 type='slow' -3.31 % ±6.38% ±8.86% ±12.34% buffers/buffer-creation.js n=1024 len=8192 type='buffer()' 0.06 % ±2.70% ±3.77% ±5.31% buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc-fill' -1.20 % ±3.30% ±4.53% ±6.17% buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc' -1.46 % ±2.75% ±3.84% ±5.38% buffers/buffer-creation.js n=1024 len=8192 type='fast-allocUnsafe' 1.27 % ±4.69% ±6.49% ±8.98% buffers/buffer-creation.js n=1024 len=8192 type='slow-allocUnsafe' -1.68 % ±3.30% ±4.62% ±6.49% buffers/buffer-creation.js n=1024 len=8192 type='slow' -2.49 % ±3.24% ±4.44% ±6.07% (Re-running the outlier with 30 runs instead of 10:) buffers/buffer-creation.js n=1024 len=4096 type='slow-allocUnsafe' 2.06 % ±2.39% ±3.19% ±4.15% The performance gains effect are undone once native code accesses the underlying ArrayBuffer, but then again that a) does not happen for all TypedArrays, and b) it should also make sense to look into using `ArrayBufferView::CopyContents()` in some places, which is made specifically to avoid such a performance impact and allows us to use the benefits of heap-allocated typed arrays. Refs: 16f86d6 Refs: #2893 Refs: 74178a5#commitcomment-13250880 Refs: http://logs.libuv.org/node-dev/2015-09-15 PR-URL: #26301 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

Where appropriate, use a helper that wraps around `ArrayBufferView::Buffer()` or `ArrayBufferView::CopyContents()` rather than `Buffer::Data()`, as that may help to avoid materializing the underlying `ArrayBuffer` when reading small typed arrays from C++. This allows keeping the performance benefits of the faster creation of heap-allocated small typed arrays in many cases. PR-URL: #26301 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

Do not create an `ArrayBuffer` if the engine’s settings avoid it and we don’t need it. PR-URL: #26301 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

Check that small typed arrays, including `Buffer`s (unless allocated by `Buffer.allocUnsafe()`), are indeed heap-allocated. PR-URL: #26301 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

addaleax · 2019-03-01T21:16:49Z

Here’s CITGM on v11.x-staging after this has landed on it: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1755/

This was added in 16f86d6, based on the assumption that otherwise, the memory behind `ArrayBuffer` instances could be moved around on the heap while native code holds references to it. This does not match what V8 actually does (and also did at the time): - The option/build variable was about always only about TypedArrays, not ArrayBuffers. Calls like `new ArrayBuffer(4)` call into C++ regardless of the option value, but calls like `new Uint8Array(4)` would not call into C++ under V8 defaults. - When first accessing a heap-allocated TypedArray’s `ArrayBuffer`, whether that is through the JS `.buffer` getter or the C++ `ArrayBufferView::Buffer()` function, a copy of the contents is created using the ArrayBuffer allocator and stored as the (permanent, unmovable) backing store. As a consequence, the memory returned by `ArrayBuffer::GetContents()` is not moved around, because it is fixed once the `ArrayBuffer` object itself first comes into explicit existence in any way. Removing this build option significantly speeds up creation of typed arrays from JS: $ ./node benchmark/compare.js --new ./node --old ./node-master --runs 10 --filter buffer-creation.js buffers | Rscript benchmark/compare.R confidence improvement accuracy (*) (**) (***) buffers/buffer-creation.js n=1024 len=10 type='buffer()' *** 593.66 % ±28.64% ±41.10% ±60.36% buffers/buffer-creation.js n=1024 len=10 type='fast-alloc-fill' *** 675.42 % ±90.67% ±130.24% ±191.54% buffers/buffer-creation.js n=1024 len=10 type='fast-alloc' *** 663.55 % ±58.41% ±83.87% ±123.29% buffers/buffer-creation.js n=1024 len=10 type='fast-allocUnsafe' 3.10 % ±9.63% ±13.22% ±18.07% buffers/buffer-creation.js n=1024 len=10 type='slow-allocUnsafe' 4.67 % ±5.55% ±7.77% ±10.97% buffers/buffer-creation.js n=1024 len=10 type='slow' -2.48 % ±4.47% ±6.12% ±8.34% buffers/buffer-creation.js n=1024 len=1024 type='buffer()' -1.91 % ±4.71% ±6.45% ±8.79% buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc-fill' -1.34 % ±7.53% ±10.33% ±14.10% buffers/buffer-creation.js n=1024 len=1024 type='fast-alloc' 0.52 % ±5.00% ±6.87% ±9.40% buffers/buffer-creation.js n=1024 len=1024 type='fast-allocUnsafe' 0.39 % ±5.65% ±7.78% ±10.67% buffers/buffer-creation.js n=1024 len=1024 type='slow-allocUnsafe' -0.13 % ±5.68% ±7.83% ±10.77% buffers/buffer-creation.js n=1024 len=1024 type='slow' -5.07 % ±7.15% ±9.80% ±13.35% buffers/buffer-creation.js n=1024 len=2048 type='buffer()' 0.57 % ±2.70% ±3.74% ±5.16% buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc-fill' -1.60 % ±4.96% ±6.79% ±9.25% buffers/buffer-creation.js n=1024 len=2048 type='fast-alloc' 1.29 % ±3.79% ±5.20% ±7.09% buffers/buffer-creation.js n=1024 len=2048 type='fast-allocUnsafe' 2.73 % ±8.79% ±12.05% ±16.41% buffers/buffer-creation.js n=1024 len=2048 type='slow-allocUnsafe' -0.99 % ±6.27% ±8.65% ±11.91% buffers/buffer-creation.js n=1024 len=2048 type='slow' -5.98 % ±6.24% ±8.71% ±12.20% buffers/buffer-creation.js n=1024 len=4096 type='buffer()' -1.75 % ±3.48% ±4.78% ±6.56% buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc-fill' -3.18 % ±3.97% ±5.45% ±7.45% buffers/buffer-creation.js n=1024 len=4096 type='fast-alloc' 2.05 % ±4.05% ±5.58% ±7.65% buffers/buffer-creation.js n=1024 len=4096 type='fast-allocUnsafe' 1.44 % ±5.51% ±7.63% ±10.57% buffers/buffer-creation.js n=1024 len=4096 type='slow-allocUnsafe' * -4.77 % ±4.30% ±5.90% ±8.06% buffers/buffer-creation.js n=1024 len=4096 type='slow' -3.31 % ±6.38% ±8.86% ±12.34% buffers/buffer-creation.js n=1024 len=8192 type='buffer()' 0.06 % ±2.70% ±3.77% ±5.31% buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc-fill' -1.20 % ±3.30% ±4.53% ±6.17% buffers/buffer-creation.js n=1024 len=8192 type='fast-alloc' -1.46 % ±2.75% ±3.84% ±5.38% buffers/buffer-creation.js n=1024 len=8192 type='fast-allocUnsafe' 1.27 % ±4.69% ±6.49% ±8.98% buffers/buffer-creation.js n=1024 len=8192 type='slow-allocUnsafe' -1.68 % ±3.30% ±4.62% ±6.49% buffers/buffer-creation.js n=1024 len=8192 type='slow' -2.49 % ±3.24% ±4.44% ±6.07% (Re-running the outlier with 30 runs instead of 10:) buffers/buffer-creation.js n=1024 len=4096 type='slow-allocUnsafe' 2.06 % ±2.39% ±3.19% ±4.15% The performance gains effect are undone once native code accesses the underlying ArrayBuffer, but then again that a) does not happen for all TypedArrays, and b) it should also make sense to look into using `ArrayBufferView::CopyContents()` in some places, which is made specifically to avoid such a performance impact and allows us to use the benefits of heap-allocated typed arrays. Refs: 16f86d6 Refs: #2893 Refs: 74178a5#commitcomment-13250880 Refs: http://logs.libuv.org/node-dev/2015-09-15 PR-URL: #26301 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

Where appropriate, use a helper that wraps around `ArrayBufferView::Buffer()` or `ArrayBufferView::CopyContents()` rather than `Buffer::Data()`, as that may help to avoid materializing the underlying `ArrayBuffer` when reading small typed arrays from C++. This allows keeping the performance benefits of the faster creation of heap-allocated small typed arrays in many cases. PR-URL: #26301 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

Do not create an `ArrayBuffer` if the engine’s settings avoid it and we don’t need it. PR-URL: #26301 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

Check that small typed arrays, including `Buffer`s (unless allocated by `Buffer.allocUnsafe()`), are indeed heap-allocated. PR-URL: #26301 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

addaleax added buffer Issues and PRs related to the buffer subsystem. dont-land-on-v8.x labels Feb 25, 2019

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Feb 25, 2019

addaleax added 4 commits February 25, 2019 19:55

buffer: avoid materializing ArrayBuffer for creation

6b86059

Do not create an `ArrayBuffer` if the engine’s settings avoid it and we don’t need it.

test: verify heap buffer allocations occur

3f96c4d

Check that small typed arrays, including `Buffer`s (unless allocated by `Buffer.allocUnsafe()`), are indeed heap-allocated.

addaleax force-pushed the allow-typed-arrays-on-heap branch from fcb6bb0 to 3f96c4d Compare February 25, 2019 18:56

mcollina approved these changes Feb 26, 2019

View reviewed changes

addaleax added needs-citgm PRs that need a CITGM CI run. performance Issues and PRs related to the performance of Node.js. labels Feb 26, 2019

jasnell approved these changes Feb 26, 2019

View reviewed changes

addaleax removed dont-land-on-v10.x needs-citgm PRs that need a CITGM CI run. labels Feb 28, 2019

mcollina approved these changes Mar 1, 2019

View reviewed changes

addaleax closed this Mar 1, 2019

addaleax deleted the allow-typed-arrays-on-heap branch March 1, 2019 21:16

BridgeAR mentioned this pull request Mar 4, 2019

v11.11.0 proposal #26322

Merged

jorangreef mentioned this pull request Mar 8, 2019

napi_get_buffer_info(): change of behavior in v11.11.0 #26514

Closed

BethGriggs added the backport-requested-v10.x label Apr 16, 2019

puzpuzpuz mentioned this pull request Oct 7, 2020

Avoid unnecessary Buffer allocations in Node.js axios/axios#3321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src,buffer: allow heap-allocated typed arrays #26301

src,buffer: allow heap-allocated typed arrays #26301

addaleax commented Feb 25, 2019 •

edited

Loading

nodejs-github-bot commented Feb 25, 2019

psmarshall commented Feb 26, 2019

mcollina left a comment

addaleax commented Feb 26, 2019

mcollina commented Feb 26, 2019

trevnorris commented Feb 27, 2019 •

edited

Loading

addaleax commented Feb 28, 2019

jeisinger commented Feb 28, 2019

addaleax commented Feb 28, 2019 •

edited

Loading

addaleax commented Mar 1, 2019

mcollina left a comment

addaleax commented Mar 1, 2019

addaleax commented Mar 1, 2019

src,buffer: allow heap-allocated typed arrays #26301

src,buffer: allow heap-allocated typed arrays #26301

Conversation

addaleax commented Feb 25, 2019 • edited Loading

build: remove v8_typed_array_max_size_in_heap option

src: allow not materializing ArrayBuffers from C++

buffer: avoid materializing ArrayBuffer for creation

test: verify heap buffer allocations occur

Checklist

nodejs-github-bot commented Feb 25, 2019

psmarshall commented Feb 26, 2019

mcollina left a comment

Choose a reason for hiding this comment

addaleax commented Feb 26, 2019

mcollina commented Feb 26, 2019

trevnorris commented Feb 27, 2019 • edited Loading

addaleax commented Feb 28, 2019

jeisinger commented Feb 28, 2019

addaleax commented Feb 28, 2019 • edited Loading

addaleax commented Mar 1, 2019

mcollina left a comment

Choose a reason for hiding this comment

addaleax commented Mar 1, 2019

addaleax commented Mar 1, 2019

addaleax commented Feb 25, 2019 •

edited

Loading

trevnorris commented Feb 27, 2019 •

edited

Loading

addaleax commented Feb 28, 2019 •

edited

Loading