1108444 - Setting __proto__ interacts badly with addprop ICs

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Shumway's raytrace.swf benchmark has a bunch of constructors. For each setprop in these constructors, we attach an add-property stub. These stubs shape-guard on every object on the proto chain.

Then some unrelated function does:

    parameterizedVector.__proto__ = GenericVector;

This ends up generating new shapes for all objects on parameterizedVector's proto. Pretty much all objects have Object.prototype on the proto chain, so all add-property stubs become useless. We keep doing this and quickly reach the max number of stubs.

Bug 1107515 will hopefully help here, but it's still pretty unfortunate. Setting __proto__ is known to be performance sensitive, but it's really unfortunate it affects completely unrelated objects and code.

Comment 1

[Jan de Mooij [:jandem]]

To be clear, I'm filing this because it likely affects other code as well. Shumway should really stop setting __proto__, but there are probably lots of websites that do something similar.

Comment 2

[Till Schneidereit [:till]]

(In reply to Jan de Mooij [:jandem] from comment #1)
> To be clear, I'm filing this because it likely affects other code as well.
> Shumway should really stop setting __proto__, but there are probably lots of
> websites that do something similar.

Agreed on both counts. We'll look into finally removing our usages, but it's not easy to do: We have constructors that leak to various places before we even really get to start running our own logic. These are what TypeScript compiles its classes to, and are directly available as variables to other classes in the same module. Then at a later point, our runtime kicks in and we have to modify these constructors to have certain objects on their prototype chain. Ideally, we wouldn't modify but but just replace them wholesale, with the constructor function being .call'd on new instances. That would largely work fine, but it's extremely hard to get right and not introduce subtle bugs in the process.

In a way it's good to finally have proof of this really hurting our performance, I guess. That it actually does hurt our performance and is hard to fix isn't so nice, obviously.