Closed Bug 1944081 Opened 23 days ago Closed 8 days ago

Experiment with preserving shape teleporting in the presence of reshape for mutation.

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
137 Branch
Tracking Status
firefox137 --- fixed

People

(Reporter: mgaudet, Assigned: mgaudet)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

See Bug 1936586 for context

The more correct version of this would be to somehow create a new shape in ReshapeForFoo without setting a flag. The problem is that currently we effectively define (non-dictionary) shapes by-value, so if we're not changing a flag, then we expect to get the same shape back from the initial shapes cache. We might be able to make it work by removing the entry from the cache (and the one-entry cache on its own proto?), but that's a very delicate change that would require careful auditing.

Severity: -- → S3
Priority: -- → P3

(Built a non-landable version of this which helps for some cases; looking into building something more robust)

Assignee: nobody → mgaudet

(Just a random pointer: Using the demo app provided, the Bryntum page hits 1673 shadowed property reshapes. This means capping the number of property reshapes probably should be a bigger number than I would have initially chosen. )

This patch aims to mitigate a performance cliff that occurs when we have applications
which shadow properties on the prototype chain or which mutate the prototype chain.
The problem is that these actions currently break a property lookup optimization
called "Shape Teleportation"

Shape Teleporting is the existing optimization which says that so long as you
actively force a change of shape on objects in the prototype chain when certain
modifications occur, then you can guard in inline-caches only on the shape of the
receiver object and the shape of the holder object.

By forcing each shape to be changed, inline caches which have baked in assumptions
about these objects will no longer succeed, and we'll take a slow path, potentially
attaching a new IC if possible.

We must reshape in the following situations:

  1. Adding a property to a prototype which shadows a property further up the prototype
    chain. In this circumstance, the object getting the new property will naturally
    reshape to account for the new property, but the old holder needs to be explicitly
    reshaped at this point, to avoid an inline cache jumping over the newly defined
    prototype.

  2. Modifying the prototype of an object which exists on the prototype chain. For this
    case we need to invalidate the shape of the object being mutated (natural reshape due
    to changed prototype), as well as the shapes of all objects on the mutated object’s
    prototype chain. This is to invalidate all stubs which have teleported over the
    mutated object.

Furthermore, we must avoid an "A-B-A" problem, where an object returns to a shape
prior to prototype modification.

Prior to this patch, Watchtower watches for prototype mutation and shadowing, and
marks the shapes of the prototype objects involved with these operations as
InvalidatedTeleporting. This means that property access with the objects involved
can never more rely on the shape teleporting optimization. This also avoids the A-B-A
problem as new shapes will always carry along the InvalidatedTeleporting flag.

This patch instead chooses to migrate an object shape to dictionary mode, or generate
a new dictionary shape if it's already in dictionary mode. Using dictionary mode
shapes works because all dictionary mode shapes are unique and never recycled. This
ensures the ICs are no longer valid as expected, as well as handily avoiding the
A-B-A problem.

The patch does keep the InvalidatedTeleporting flag to catch potentially ill-behaved
sites that do lots of mutation and shadowing, avoiding having to reshape proto
objects forever.

The patch also provides a preference to allow cross-comparison between old and new,
however this patch defaults to dictionary mode teleportation.

Performance testing on micro-benchmarks shows large impact by allowing ICs to attach
where they couldn't before, however Speedometer3 shows no real movement.

Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c2c84c3ec0eb Bring back dictionary-mode based shape teleportation r=jandem
Status: NEW → RESOLVED
Closed: 8 days ago
Resolution: --- → FIXED
Target Milestone: --- → 137 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: