path: change path.join to use array dynamic allocation

[HBSPS]

Change path.join to use array dynamic allocation instead of push.

                                                               confidence improvement accuracy (*)   (**)  (***)
path/join-posix.js n=100000 paths='/foo|bar||baz/asdf|quux|..'                 0.19 %       ±0.53% ±0.71% ±0.92%

Refs: #54331

[aduh95]

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1606/

                                                               confidence improvement accuracy (*)   (**)  (***)
path/join-posix.js n=100000 paths='/foo|bar||baz/asdf|quux|..'                -0.15 %       ±0.75% ±1.00% ±1.30%

[aduh95]

I the benchmark results you shared can be generalized, we should probably replace all instances of ArrayPrototypePush with this instead.

[targos]

Why would we do that?

[targos]

If we don't care about prototype pollution, array.push() is more straightforward IMO.

[HBSPS]

If we don't care about prototype pollution, array.push() is more straightforward IMO.

I agree with you.

I think this could be a solution for those who are worried about prototype pollution and those who are focusing on performance, although other points will need to check more.

This is because there is no significant difference in performance from array.push and does not use built-in object methods.

[targos]

But:

• array[array.length] = value is also subject to prototype pollution
• the benchmarks show that it's not faster

[aduh95]

• array[array.length] = value is also subject to prototype pollution

Just as much as ArrayPrototypePush, so if it has better performance, it just makes sense to use it.

[targos]

My point is that it doesn't have better performance than array.push(). What's the reason to use it now that we have the benchmark results?

[aduh95]

I don't disagree with your point. Let me try to better explain my perspective:

Method	Performance	Reliance on %Array.prototype%.push	Reliance on %Array.prototype% not having number properties
require("internal/util").setOwnProperty(arr, arr.length, …)	🐌	👍	👍
ArrayPrototypePush(arr, …)	🐢	👍	❌
arr[arr.length] = …	👍	👍	❌
arr.push(…)	👍	❌	❌

My point is that we should use arr[arr.length] = … because that seems to be the one balancing best performance and "security" (equivalent to ArrayPrototypePush for prototype pollution, and equivalent to arr.push(…) for performance).

[targos]

Are you sure that ArrayPrototypePush is slower? It's not what #54331 (comment) shows.
I personally prefer keeping ArrayPrototypePush if the "safety" is the same. It is more readable IMO.