Closed
Bug 1357468
Opened 7 years ago
Closed 7 years ago
Optimize Object.hasOwnProperty on Speedometer
Categories
(Core :: JavaScript Engine: JIT, enhancement)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: evilpie, Assigned: evilpie)
References
Details
Attachments
(2 files)
11.96 KB,
patch
|
evilpie
:
review+
|
Details | Diff | Splinter Review |
13.35 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
This patch was originally in bug 1344469, but I wanted to file a new bug to track this. Basically with those patches we can cache all hasOwnProperty calls, but of course some of them are still megamorphic, so patches like in bug 1356315 will help. This patch adds two additional features: - Support for unboxed objects, which is a bit unclean by extending the native case, but this will be fixed soon. - Dense elements, i.e code like [0].hasOwnProperty("0"). This happens often with for .. in loops. This patch was reviewed by Jan in bug 1344469 comment 37.
Attachment #8859256 -
Flags: review+
Assignee | ||
Comment 1•7 years ago
|
||
Oh sorry, I forgot about doing hasOwnProperty on Proxy objects, which is important for various DOM objects like NodeList etc.
Pushed by evilpies@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d505fbfbd4e2 More Object.hasOwnProperty optimizations for Speedometer. r=jandem
Comment 3•7 years ago
|
||
Backed out for asserting an e.g. failing js/src/jit-test/tests/arrays/sort-getter-only.js: https://hg.mozilla.org/integration/mozilla-inbound/rev/30e2910469f720c43bd913bb02fcb7ded4d19cc4 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=89375918b96e056c339f8845c919361ac52bc91f&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=92409199&repo=mozilla-inbound [task 2017-04-18T17:06:37.604931Z] TEST-PASS | js/src/jit-test/tests/arrow-functions/church-1.js | Success (code 0, args "") [task 2017-04-18T17:06:37.611541Z] Assertion failure: hasValue(), at /home/worker/workspace/build/src/js/src/jit/RegisterSets.h:210 [task 2017-04-18T17:06:37.611625Z] Exit code: -11 [task 2017-04-18T17:06:37.611682Z] FAIL - arrays/sort-getter-only.js [task 2017-04-18T17:06:37.611857Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/arrays/sort-getter-only.js | Assertion failure: hasValue(), at /home/worker/workspace/build/src/js/src/jit/RegisterSets.h:210 (code -11, args "--ion-eager --ion-offthread-compile=off --non-writable-jitcode --ion-check-range-analysis --ion-extra-checks --no-sse3 --no-threads") [task 2017-04-18T17:06:37.611904Z] INFO exit-status : -11 [task 2017-04-18T17:06:37.611953Z] INFO timed-out : False [task 2017-04-18T17:06:37.612018Z] INFO stderr 2> Assertion failure: hasValue(), at /home/worker/workspace/build/src/js/src/jit/RegisterSets.h:210 [task 2017-04-18T17:06:37.619370Z] Assertion failure: hasValue(), at /home/worker/workspace/build/src/js/src/jit/RegisterSets.h:210 [task 2017-04-18T17:06:37.619455Z] Exit code: -11
Flags: needinfo?(evilpies)
Assignee | ||
Comment 4•7 years ago
|
||
In CacheIRCompiler::emitLoadDenseElementExistsResult we were using output.valueReg(), but in Ion this might be typed as boolean. So we have to use typedReg()
Flags: needinfo?(evilpies)
Attachment #8859359 -
Flags: review?(jdemooij)
Assignee | ||
Comment 5•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac754ccabc7e0b992e54e53a53a86248ffc17e43&selectedJob=92457622
Comment 6•7 years ago
|
||
Comment on attachment 8859359 [details] [diff] [review] v2 Review of attachment 8859359 [details] [diff] [review]: ----------------------------------------------------------------- I only looked at the CacheIRCompiler.cpp changes. We could rename LoadDenseElementExistsResult to GuardDenseElementExists and then emit LoadBooleanResult(true), but it doesn't really matter. ::: js/src/jit/CacheIRCompiler.cpp @@ +1937,5 @@ > + if (output.hasValue()) { > + masm.moveValue(BooleanValue(true), output.valueReg()); > + } else { > + masm.movePtr(ImmWord(true), output.typedReg().gpr()); > + } Nit: no {}, or maybe add MOZ_ASSERT(output.type() == JSVAL_TYPE_BOOLEAN) to the else branch.
Attachment #8859359 -
Flags: review?(jdemooij) → review+
Pushed by evilpies@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d555db01bb95 More Object.hasOwnProperty optimizations for Speedometer. r=jandem
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d555db01bb95
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•