Last Comment Bug 1328076 - CacheIR: Attach primitive IC even for non-existant properties
: CacheIR: Attach primitive IC even for non-existant properties
Product: Core
Classification: Components
Component: JavaScript Engine: JIT (show other bugs)
: unspecified
: Unspecified Unspecified
-- normal with 1 vote (vote)
: mozilla53
Assigned To: Tom Schuster [:evilpie]
: Hannes Verschore [:h4writer]
Depends on:
Blocks: CacheIR
  Show dependency treegraph
Reported: 2017-01-01 04:59 PST by Tom Schuster [:evilpie]
Modified: 2017-01-05 00:43 PST (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

WIP (1.31 KB, patch)
2017-01-01 04:59 PST, Tom Schuster [:evilpie]
no flags Details | Diff | Splinter Review
v1 with tests (2.40 KB, patch)
2017-01-03 09:26 PST, Tom Schuster [:evilpie]
jdemooij: review+
Details | Diff | Splinter Review

Description User image Tom Schuster [:evilpie] 2017-01-01 04:59:15 PST
Created attachment 8822942 [details] [diff] [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.
Comment 1 User image Tom Schuster [:evilpie] 2017-01-03 09:26:35 PST
Created attachment 8823340 [details] [diff] [review]
v1 with tests

This allows us the cache properties on the whole primitive proto chain and also cache non existing properties.
Comment 2 User image Jan de Mooij [:jandem] 2017-01-03 10:20:15 PST
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.
Comment 3 User image Pulsebot 2017-01-04 03:08:23 PST
Pushed by
Attach primitive IC for more properties. r=jandem
Comment 4 User image Carsten Book [:Tomcat] 2017-01-04 07:17:17 PST
Comment 5 User image Guilherme Lima 2017-01-04 18:15:19 PST
This patch (or 'Remove Proxy friendapi' from bug 1323190) improved PDFJS by 9% and regressed GameBoy by 3% -
Comment 6 User image Jan de Mooij [:jandem] 2017-01-04 23:52:29 PST
react-shell also got about 1% faster:
Comment 7 User image Hannes Verschore [:h4writer] 2017-01-05 00:43:32 PST
== Change summary for alert #4715 (as of January 04 2017 11:27 UTC) ==


 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:

Note You need to log in before you can comment on or make changes to this bug.