CacheIR: Attach primitive IC even for non-existant properties

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
2 years ago
2 years 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

2 years ago
Posted patch WIP (obsolete) — Splinter Review
Scrolling twitter shows that they try to lookup 'nodeType' and other jQuery properties on primitive strings, I think we have seen this before in bug 1106100. jQuery uses this to infer what type of value they are dealing with.

We can just use our whole CanAttachNativeGetProp with ReadSlot machinery on the proto, i.e. String.prototype. This allows us to IC non-existant properties as well as properties that aren't directly on the prototype.
(Assignee)

Comment 1

2 years ago
Posted patch v1 with testsSplinter Review
This allows us the cache properties on the whole primitive proto chain and also cache non existing properties.
Assignee: nobody → evilpies
Status: NEW → ASSIGNED
Attachment #8823340 - Flags: review?(jdemooij)
Comment on attachment 8823340 [details] [diff] [review]
v1 with tests

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

Excellent! Thanks for adding the tests.

::: js/src/jit/CacheIR.cpp
@@ +858,2 @@
>      if (IsIonEnabled(cx_))
>          EnsureTrackPropertyTypes(cx_, proto, id);

We should call this on |holder| now, if it's non-null.
Attachment #8823340 - Flags: review?(jdemooij) → review+
(Assignee)

Updated

2 years ago
Attachment #8822942 - Attachment is obsolete: true

Comment 3

2 years ago
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6ac89909ecc
Attach primitive IC for more properties. r=jandem

Comment 4

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a6ac89909ecc
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Comment 5

2 years ago
This patch (or 'Remove Proxy friendapi' from bug 1323190) improved PDFJS by 9% and regressed GameBoy by 3% - https://arewefastyet.com/#machine=29&view=breakdown&suite=octane
== Change summary for alert #4715 (as of January 04 2017 11:27 UTC) ==

Improvements:

 12%  octane 2.0.1 PdfJS linux64 opt shell     12072.46 -> 13570
 11%  octane 2.0.1 PdfJS linux32 opt shell     12560.92 -> 13947.92
  6%  jetstream 1.0 pdfjs linux64 opt          86.67 -> 91.73

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=4715
You need to log in before you can comment on or make changes to this bug.