doc: deprecate buffer.slice

[benjamingr]

Opening a PR to have the discussion about this particular idea (doc-deprecating buffer.slice).

References: #41588

For the why - buffer.slice() returns a mutable slice whereas typedArray.slice() and array.slice() return a copy. This is confusing behavior and is the result of buffers predating Uint8Array availability.

cc @addaleax @jasnell @sindresorhus @nodejs/buffer

Also ccing some people who are usually helpful at giving useful comments when I make PRs that touch docs: @Trott @aduh95 @VoltrexMaster

[Mesteery]

An entry in https://github.com/nodejs/node/blob/master/doc/api/deprecations.md should also be added.

[benjamingr]

Done, thanks.

[vweevers]

Maybe relevant: I noticed that buffer.subarray() is slower than buffer.slice() (I think it was on node 16.9.1)

[jasnell]

Generally +1 on this. The one downside is that whenever a Buffer is used transparently as a Uint8Array users could still end up using slice() without realizing that there's a material difference between those. It might make sense eventually to make this a runtime deprecation or emit a warning.

[benjamingr]

@aduh95 TIL that issue titles support markdown :O

@vweevers if that is a concern we can override subarray with slice however I think that's surprising looking at the code (it implies new Uint8Array(arr.buffer, byteOffset, length) is measurably faster than arr.subarray(begin, end) (that's basically what the .slice code does).

(Edit, looking at V8 this is subarray https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/typed-array-subarray.tq?q=subarray&ss=chromium%2Fchromium%2Fsrc and here is some of the logic https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/typed-array-subarray.tq?q=subarray&ss=chromium%2Fchromium%2Fsrc if this is a concern we can run benchmarks)

[benjamingr]

@jasnell Ideally I'll follow this up with a runtime warning if --pending-deprecations is run but I think a warning is likely going to be hard to ship (I may be pessimistic) given how omnipresent buffers are.

[mcollina]

I'm -1 for this deprecation. buffer.slice() is used kind of everywhere.

[aduh95]

I believe the subsystem in the commit message should doc: as this is a doc-deprecation.

If proposal-rm-builtin-subclassing ever goes through, buffer.subarray would no longer return a Buffer (it would return a UInt8Array), not sure if this is actually a concern but maybe something to have in mind.

I've also noticed performance regressions when trying to replace .slice calls with primordials.TypedArrayPrototypeSubArray, as if the Node.js slice method takes shortcut that built-in subarray doesn't take, maybe an issue with V8 🤷‍♂️

[mcollina]

I think .subarry() should have equivalent performance of slice() before deprecating slice().

[addaleax]

I'm -1 for this deprecation. buffer.slice() is used kind of everywhere.

Which is a good reason not to runtime-deprecate this, but how does the frequency of use affect doc deprecations?

[mcollina]

LGTM for a doc-deprecation but hard -1 for anything further. We would need to support this forever.

[addaleax]

I think .subarry() should have equivalent performance of slice() before deprecating slice().

I’m wondering how bad it would be if we did Buffer.prototype.subarray = Buffer.prototype.slice;? I think it would break in the case that Buffer is being subclassed, but that seems to be an edge case that we could get away with breaking.

[benjamingr]

Hey, just pre-emptively putting "blocked" on this so we don't land this without checking that the performance doesn't regress with .subarray (and address it if it does) as that concern has been raised by several people (and I also personally agree with it).

I don't think we can ever runtime deprecate it either but we can print a warning with --pending-deprecations (in a follow up) right?

[jasnell]

I’m wondering how bad it would be if we did Buffer.prototype.subarray = Buffer.prototype.slice;?

Why not flip that... make Buffer.prototype.slice an alias for Buffer.prototype.subarray, and make subarrays implementation the faster one. Attaching the alias to the prototype should address the potential subclass issue.

[addaleax]

@jasnell Sure, that’s the same thing. It doesn’t really matter whether it’s Buffer.prototype.slice = Buffer.prototype.subarray = function() { ... }; or Buffer.prototype.subarray = Buffer.prototype.slice = function() { ... }; to me.

[aduh95]

LGTM

Whoever lands this, please update subsystem in the commit messages:

doc: deprecate `buffer.slice`

benchmark: add `subarray` to `buffer-slice`

buffer: alias `subarray` and `slice`

[shalvah]

@benjamingr Made the PR: #41628

[aduh95]

Landed in 290911b...3d23d62

[benjamingr]

@bricss

It's anyway gonna be weird to have at the same time some method deprecated, and not to be deprecated, kinda heisenfeature all the way down 😵‍💫

You mean "Node.js doubles the performance of subarray!"