965793 - Handle polymorphic property accesses better

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Octane-TypeScript has an AST class with a number of "subclasses" for the different AST nodes. This means all these nodes have a different shape and we quickly fill our ICs, even though the first X properties of all these objects have exactly the same shape and slot.

For instance, the "nodeType" property is always in slot 2, and we should be able to do something more efficient than attaching the max number of stubs and then giving up.

A while ago, Luke suggested the following idea and it would help here: when we detect this case we give the shape a reverse-map. This is an array of length slotspan that has a PropertyName* for every slot. So in our nodeType case all these different node shapes will have:

    shape->reverseMap->slots[2] == "nodeType"

Then we can attach an IC stub that does this check and it will work for all shapes that have a reverseMap attached to them.

IIRC, V8 uses a global HashTable, so that you can lookup the shape + PropertyName and get a slot or property Shape back.

Comment 1

[Luke Wagner [:luke]]

Last year a research intern investigated cases of megamorphism in SM on various weby workloads and the results also supported doing this optimization: bug 905827.  Nice to have a benchmark motivation for this as well!

Comment 2

[Till Schneidereit [:till]]

Also relevant for any other implementations of class-based inheritance system in JS, such as Shumway.

Comment 3

[Brian Hackett [Laid off!]]

Do the associated type sets for the objects fill up and deoptimize to 'object'?  How many different subclasses are there?

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Brian Hackett (:bhackett) (Vacation until early February) from comment #3)
> Do the associated type sets for the objects fill up and deoptimize to
> 'object'?  How many different subclasses are there?

I noticed this with Baseline; Ion may be able to take advantage of the definite slots analysis, I'd have to check.

The AstWalker.prototype.walk function for instance is Ion-compiled like 13 times (invalidated a number of times due to TypeScript::SetArgument etc) so the ast.nodeType GETPROP there will hit Baseline's fallback stub at least 11000 times, IIRC.

Attaching 16 instead of 8 stubs helps a bit but does not fix it completely, and if we do that we should probably remove all stubs when we reach the max number of stubs to avoid walking that long chain all the time.