CacheIR: Attach DenseElementHole even with zero dense elements

RESOLVED FIXED in Firefox 53

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

(Blocks: 1 bug)

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 months ago
Created attachment 8822943 [details] [diff] [review]
WIP

Found on twitter again. As long as the object doesn't have indexed properties besides dense elements we should be able to attach this IC even when the initialized length is zero. This basically means there are no indexed elements anywhere on that object.
(Assignee)

Comment 1

6 months ago
Apparently there is some edge case where this is causing issues: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff705046fbbb21cd440eeb6effc09f4b273fddf9&selectedJob=65747940. Just started debugging.
(Assignee)

Comment 2

6 months ago
For some reason we are attaching the stub for CData objects (http://searchfox.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#583) which should be impossible, because that class has a getProperty ClassOps, which we check for in ClassCanHaveExtraProperties. Compiler bug?
(Assignee)

Comment 3

6 months ago
Created attachment 8823421 [details] [diff] [review]
v1 Attach dense element hole IC even with zero dense elements

Somehow I didn't spot the difference between getProperty in ClassOps and ObjectOps. 

Anyway I am asking Brian for additional review. What we are trying to accomplish here is make sure that indexed properties would either be defined as a dense element or not exist at all. This also means if the object currently has zero dense elements we have to make sure there is no other way for indexed properties to appear.
Assignee: nobody → evilpies
Attachment #8822943 - Attachment is obsolete: true
Attachment #8823421 - Flags: review?(jdemooij)
Attachment #8823421 - Flags: review?(bhackett1024)
Comment on attachment 8823421 [details] [diff] [review]
v1 Attach dense element hole IC even with zero dense elements

Review of attachment 8823421 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.
Attachment #8823421 - Flags: review?(jdemooij) → review+
Attachment #8823421 - Flags: review?(bhackett1024) → review+

Comment 5

6 months ago
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f897399fb28a
Attach dense element hole IC even with zero dense elements. r=jandem,bhackett

Comment 6

6 months ago
This patch caused a 40% improvement in https://arewefastyet.com/#machine=29&view=single&suite=misc&subtest=bugs-636096-model2d

Comment 7

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f897399fb28a
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.