Closed
Bug 1328077
Opened 7 years ago
Closed 7 years ago
CacheIR: Attach DenseElementHole even with zero dense elements
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: evilpie, Assigned: evilpie)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
2.08 KB,
patch
|
jandem
:
review+
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
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•7 years 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•7 years 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•7 years ago
|
||
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 4•7 years ago
|
||
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+
Updated•7 years ago
|
Attachment #8823421 -
Flags: review?(bhackett1024) → review+
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•7 years ago
|
||
This patch caused a 40% improvement in https://arewefastyet.com/#machine=29&view=single&suite=misc&subtest=bugs-636096-model2d
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f897399fb28a
Status: NEW → RESOLVED
Closed: 7 years 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.
Description
•