1328077 - CacheIR: Attach DenseElementHole even with zero dense elements

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Tom S [:evilpie]]

Attachment: 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.

Comment 1

[Tom S [:evilpie]]

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.

Comment 2

[Tom S [:evilpie]]

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?

Comment 3

[Tom S [:evilpie]]

Attachment: 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.

Comment 4

[Jan de Mooij [:jandem]]

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.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f897399fb28a
Attach dense element hole IC even with zero dense elements. r=jandem,bhackett

Comment 6

[Guilherme Lima]

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

Comment 7

[Phil Ringnalda (:philor)]

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