The flag is now used to guard against both cases teleporting has to watch out
for: proto changes and shadowed properties.
This lets us get rid of dictionary conversions and reshaping for ReshapeForShadowedProp.
Details
Details
- Reviewers
tcampbell - Commits
- Restricted Diffusion Commit
rMOZILLACENTRAL7a624fe10146: Bug 1717438 part 3 - Use the InvalidatedTeleporting flag also for shadowed…
rMOZILLACENTRAL95e913632f6a: Bug 1717438 part 3 - Use the InvalidatedTeleporting flag also for shadowed… - Bugzilla Bug ID
- 1717438
Diff Detail
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Lint
Lint Not Applicable - Unit
Tests Not Applicable - Build Status
Buildable 336580 Build 430651: arc lint + arc unit
Event Timeline
Comment Actions
This is a pretty nice cleanup.
Since you have the simple flag now, I wonder if it makes sense to expose as a testing function so that we can write a few tests to show this behaviour (and we don't even need JITs).
One impact of this change is that shadowing the ObjectPrototype will now cause future accesses to common properties to not use teleporting anymore. This was always the can for SetProto invalidations, but now is expanded to shadowing. In practice, I think the avoidance of dictionary mode here outweighs this, and the invalidation flag has much more gradual JIT impact.
A potential thing we could do if we are concerned about in-the-wild behaviour is to capture telemetry when tearing down the realm about if ObjectPrototype has set the flag.
Comment Actions
Testing function is a good idea, I posted D119063.
Regarding Object.prototype: it helps that we only check the flag on the holder object. I don't think Object.prototype has very hot properties except for toString and hasOwnProperty, so hopefully it would still be okay if we happen to invalidate teleporting for it.
| js/src/jit/CacheIR.cpp | ||
|---|---|---|
| 698 | @tcampbell: putting this back in your queue. This was backed out because of a silly bug I should have seen: if we reuse the flag for shadowing properties instead of reshaping the holder each time, this must now do a shape guard instead of a proto guard so that we properly detect shadowing properties after the first one. See test1 in the test case for the buggy scenario. I think this should be okay: shape guards are faster than proto guards, and falling back to shape guards when teleporting doesn't work is more robust and more obviously correct. | |
| js/src/jit/CacheIR.cpp | ||
|---|---|---|
| 698 | Ah, I missed this as well. This is much nicer code anyways, the previous code was quite tricky here so simplifing to just a shape-guard is welcome. | |