Array splice is not optimized when Array.prototype has been altered in v8 7.1+

[Guillaume Grossetie]

Hello,

I'm working on the Opal project and I've noticed that Array#splice is not optimized anymore (ie. 4-8 times slower) when using a recent version of v8.

To reproduce this slowdown I'm using the following code:

const arr = []

for (let j = 0; j < 50000; j++) {

  arr.push('index' + j)

}

Object.setPrototypeOf(Array.prototype, {})

let r

for (let i = 0; i < 1000; i++) {

  r = arr.splice(0, 1)

}

console.log(r)

You can run the above code against Node 10, which is using v8 version < 7, and against Node 12, which is using v8 version > 7.8.

Please note that removing "Object.setPrototypeOf(Array.prototype, {})" will effectively enable the optimization.

I think the reason is that Array#splice is now using a Torque implementation where "fast" splice is only used when Array.prototype is pristine :

https://github.com/v8/v8/blob/4b9b23521e6fd42373ebbcb20ebe03bf445494f9/src/builtins/array-splice.tq#L125

I'm well aware that we should not mess with prototypes[2][3] but is there any workaround to get an optimized version of splice even if prototype has been altered?

Or do you have any idea on how we could mitigate this issue?

Thanks for your help,

Guillaume

[1] https://github.com/opal/opal

[2] https://mathiasbynens.be/notes/prototypes

[3] https://mathiasbynens.be/notes/shapes-ics

[Ben Noordhuis]

I don't think so.

Array.p.splice() returns a new array. array-splice.tq has a fast path
for that (ExtractFastJSArray, defined in
src/codegen/code-stub-assembler.cc) but that's only sound when
Array.prototype is unmodified, hence the
IsPrototypeInitialArrayPrototype() check.

Perhaps V8 could be made smart enough to distinguish between benign
and non-benign prototype tampering but the subset that can be proven
safe is probably so small that it's not worth the complexity and
overhead.

[Guillaume Grossetie]

Thanks for your answer Ben.

> Perhaps V8 could be made smart enough to distinguish between benign

> and non-benign prototype tampering but the subset that can be proven
> safe is probably so small that it's not worth the complexity and
> overhead.

Yes, I agree.

Alternatively, could v8 use the pre 7.1 impl of Array#splice when the prototype has been modified?

Obviously, this is easier said than done and I'm guessing that it might not be an easy task.

But it's worth noting that the pre 7.1 impl performance (with and without prototype tampering) were similar.

Not sure how it was all made possible... but it was great for us :)

Anyway thanks again for your input!

Have a nice day

[Ben Noordhuis]

On Thu, Sep 24, 2020 at 11:39 AM Guillaume Grossetie
<gross...@gmail.com> wrote:
>
> Thanks for your answer Ben.
>
> > Perhaps V8 could be made smart enough to distinguish between benign
> > and non-benign prototype tampering but the subset that can be proven
> > safe is probably so small that it's not worth the complexity and
> > overhead.
>
> Yes, I agree.
> Alternatively, could v8 use the pre 7.1 impl of Array#splice when the prototype has been modified?
> Obviously, this is easier said than done and I'm guessing that it might not be an easy task.
> But it's worth noting that the pre 7.1 impl performance (with and without prototype tampering) were similar.
> Not sure how it was all made possible... but it was great for us :)
>
> Anyway thanks again for your input!
> Have a nice day

Older versions of V8 called out to this JS-with-extensions code:
https://github.com/v8/v8/blob/7.0.1/src/js/array.js#L599-L642 - I
don't speak for the V8 team but I don't believe they're looking to
return to that approach.

If Array.p.splice() performance is vital to you, you can try
open-coding it. I did something similar in Node.js a few years ago, to
great effect: https://github.com/nodejs/node/commit/d3f8db124463e478420d8bba6fa5cf13af7b4ecb

[Guillaume Grossetie]

That's extremely useful, thanks!

I will take a closer at array.js to understand how "%directive" are resolved. 

> f Array.p.splice() performance is vital to you, you can try
> open-coding it. I did something similar in Node.js a few years ago, to
> great effect: https://github.com/nodejs/node/commit/d3f8db124463e478420d8bba6fa5cf13af7b4ecb

Aha, it was you... I'm already experimenting with your spliceOne function in Opal to improve the performance :)

Many thanks!