doc: buffer.from will return internal pool buffer

[mritunjayz]

Checklist

#22139
Buffer.from will also use internal Buffer pool if it's size is less then Buffer.poolSize.

• documentation is changed or added
• commit message follows commit guidelines

[mritunjayz]

ping @ChALkeR

[mritunjayz]

@ChALkeR can you please review this ? 😊

[mritunjayz]

@ChALkeR I removed length condition from documentation.

[mritunjayz]

@Trott

[Trott]

@nodejs/buffer @nodejs/documentation

[ChALkeR]

This is not always correct. It may return 0, and any subsequent call will probably increase it.
So it doesn't always print 344.

[ChALkeR]

This could be misleading. Changing Buffer.poolSize does have immediate effect on which buffers would be pooled, but it doesn't immediately change the size of the current pool, it changes the size of the next pool.
It does, however, take immediate effect for determining whether should the buffer be allocated from the pool or not.

[ChALkeR]

@mritunjaygoutam12 Sorry for delay on this, that was mostly due to my personal issues :-/.
I am available and will respond more actively on this from now on.

---

Overall, I don't think that the code example is very useful -- it's the confusing part atm, as it seems for me. Perhaps we can get this to land faster without the code example at all.

As for the remaining part -- it specifically mentions Buffer.from(array) for some reason, when in fact many other buffer operations are pooled.

E.g. these are not covered:

> Buffer.from('sdf').offset
280
> Buffer.concat([Buffer.from('xxx'), Buffer.from('yyy')]).offset
304
> Buffer.from(Buffer.from('sdf')).offset
320
> Buffer.from(new String('test')).offset
328

The problem of the current documentation in master is that this section:

node/doc/api/buffer.md

Lines 100 to 103 in 17c6b1d

	`Buffer` instances returned by [`Buffer.allocUnsafe()`] *may* be allocated off
	a shared internal memory pool if `size` is less than or equal to half
	[`Buffer.poolSize`]. Instances returned by [`Buffer.allocUnsafeSlow()`] *never*
	use the shared internal memory pool.

makes it seem that only .allocUnsafe is pooled, when in fact mostly everything is pooled.

Same for:

node/doc/api/buffer.md

Lines 597 to 609 in 17c6b1d

	Note that the `Buffer` module pre-allocates an internal `Buffer` instance of
	size [`Buffer.poolSize`] that is used as a pool for the fast allocation of new
	`Buffer` instances created using [`Buffer.allocUnsafe()`] and the deprecated
	`new Buffer(size)` constructor only when `size` is less than or equal to
	`Buffer.poolSize >> 1` (floor of [`Buffer.poolSize`] divided by two).
	Use of this pre-allocated internal memory pool is a key difference between
	calling `Buffer.alloc(size, fill)` vs. `Buffer.allocUnsafe(size).fill(fill)`.
	Specifically, `Buffer.alloc(size, fill)` will *never* use the internal `Buffer`
	pool, while `Buffer.allocUnsafe(size).fill(fill)` *will* use the internal
	`Buffer` pool if `size` is less than or equal to half [`Buffer.poolSize`]. The
	difference is subtle but can be important when an application requires the
	additional performance that [`Buffer.allocUnsafe()`] provides.

Perhaps some of the rewording should touch those?

Also, /cc @nodejs/tsc @nodejs/buffer @nodejs/security-wg -- what was the reason for making Buffer.alloc non-pooled, but Buffer.from pooled? Buffer.alloc could also benefit from pre-allocating zero-filled buffers, can't it? If providing pooled buffers is an issue, we shouldn't do that for everything. If it's not, perhaps we should just pool Buffer.alloc in a semver-major, so users won't be punished for using it instead of Buffer.allocUnsafe().fill()?

[vsemozhetbyt]

Some nits for consistency with other examples.

[vsemozhetbyt]

Suggested change

	// prints: 344
	// Prints: 344

[vsemozhetbyt]

Suggested change

	// prints: 8192
	// Prints: 8192

[vsemozhetbyt]

Suggested change

	// prints: 0
	// Prints: 0

[gireeshpunathil]

@mritunjayz - can you address the review comments?

[HarshithaKP]

@mritunjayz, are you still interested in working on the PR ? If not, let me know; I can pick this up and progress.

[mritunjayz]

@HarshithaKP yes, you can. Thanks

[HarshithaKP]

This can be closed as the intention is fulfilled in #32703.