1357680 - Babel class inheritance code confuses TI and Ion inlining

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P1)

Description

[Jan de Mooij [:jandem]]

Ember's Glimmer VM is transpiled with Babel. For derived classes, Babel uses setPrototypeOf on the constructor function. This pattern confuses TI and hence we don't inline some trivial base constructors and end up with megamorphic property accesses on the Ember benchmark in bug 1352486.

I have patches to fix this and some related issues with proto mutation,

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Allow inlining functions with unknown properties

Ion is currently unable to inline functions with unknown properties, because we need to add a type constraint to trigger invalidation of compilations that inlined the script when the callees stack types change.

This patch replaces this constraint with a Vector of RecompileInfo entries (compilations that inlined this script) on TypeScript, so we can invalidate them on type changes.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a446a56598f
part 1 - Track Ion-inlined scripts explicitly so we can inline functions with unknown properties. r=bhackett

Comment 3

[Iris Hsiao [:ihsiao]]

sorry had to back this out for leak issue like https://treeherder.mozilla.org/logviewer.html#?job_id=93241968&repo=mozilla-inbound&lineNumber=9557

Comment 4

[Jan de Mooij [:jandem]]

Ugh a js_free must be js_delete, I had that change at some point but must have dropped it.

Comment 5

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ccc9c855766
Backed out changeset 7a446a56598f for leak issue

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/954eeda43262
part 1 - Track Ion-inlined scripts explicitly so we can inline functions with unknown properties. r=bhackett

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/954eeda43262

Comment 8

[Jan de Mooij [:jandem]]

Relanded and stuck.

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Remove unnecessary setNewGroupUnknown in SetClassAndProto

This patch is also unlikely to improve anything but it's another incremental step.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c35f0ad34397
part 2 - Remove unnecessary setNewGroupUnknown call in SetClassAndProto. r=bhackett

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c35f0ad34397

Comment 12

[Jan de Mooij [:jandem]]

Attachment: Part 3 - TI changes

This patch makes 2 changes:

(1) When we mutate the prototype of a native object, instead of marking the new group as having unknown properties, we propagate the property types from the object to the new group.

(2) When we mutate the prototype of a scripted function, we create a new group and propagate the canonical JSFunction to it.

On the Ember benchmark we can inline more functions now and I hope these changes will make us more robust in a lot of other cases that involve proto mutation.

Comment 13

[Jan de Mooij [:jandem]]

Comment on attachment 8863358 [details] [diff] [review]
Part 3 - TI changes

Brian has no internet access for a while but reviewed this over email.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6315d186b4b
part 3 - Don't mark the new group as having unknown properties when changing an object's proto. r=bhackett

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b6315d186b4b

Comment 16

[:Harald Kirschner :digitarald]

This will affect speedometer with updated Ember. marking [qf:p1] for tracking high impact improvements.