Closed
Bug 852092
Opened 12 years ago
Closed 12 years ago
Improve DOM list ICs
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: peterv, Assigned: peterv)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
We should be more clever about when we want to generate a stub. Right now, at the time that we generate the stub, if the object has an expando that shadows then we generate a stub that bails out if there's an expando. Which means that if we loop over a list that has an expando we keep bailing out and regenerating a stub until we hit the max number of stubs. I think we should just not generate a stub if we have an object with a shadowing expando.
The Ion version of the IC also doesn't have support for slot reads, which means it fails for all DOM methods.
Assignee | ||
Comment 1•12 years ago
|
||
This just factors out the code that generates the guards for the DOM lists in the Ion IC. This only moves and reindents code (and changes one comment).
Attachment #726100 -
Attachment is obsolete: true
Attachment #729143 -
Flags: review?(jdemooij)
Assignee | ||
Comment 2•12 years ago
|
||
This does two things, it stops us from generating stubs for objects that we know have an expando (so that we don't generate a stub and then immediately hit the guard and generate another stub and so on) and it make the Ion IC work for slot reads on the prototypes of ListBase proxies (all DOM methods are stored in slots on the prototypes).
Attachment #729155 -
Flags: review?(jdemooij)
Comment 3•12 years ago
|
||
Comment on attachment 729143 [details] [diff] [review]
Part 1 v1
Review of attachment 729143 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/src/ion/IonCaches.cpp
@@ +334,5 @@
> CodeOffsetJump exitOffset;
> CodeOffsetJump rejoinOffset;
> CodeOffsetLabel stubCodePatchOffset;
>
> + void generateListBaseChecks(JSContext *cx, MacroAssembler &masm, JSObject *obj,
Nit: move this out of the class:
static void
GenerateListBaseChecks(...)
{
}
(This patch will conflict with bug 849469, which refactored the other methods.)
@@ +335,5 @@
> CodeOffsetJump rejoinOffset;
> CodeOffsetLabel stubCodePatchOffset;
>
> + void generateListBaseChecks(JSContext *cx, MacroAssembler &masm, JSObject *obj,
> + PropertyName *propName, Register object, Label &stubFailure)
Nit: use "Label *" for consistency with the other functions.
Attachment #729143 -
Flags: review?(jdemooij) → review+
Comment 4•12 years ago
|
||
Comment on attachment 729155 [details] [diff] [review]
Part 2 v1
Review of attachment 729155 [details] [diff] [review]:
-----------------------------------------------------------------
Nice.
::: js/src/ion/IonCaches.cpp
@@ +404,5 @@
> failures);
>
> + bool isCacheableListBase = IsCacheableListBase(obj);
> + Label listBaseFailures;
> + if (isCacheableListBase) {
Nit: single line body so no { }.
@@ +526,1 @@
> }
Nit: missed it in the previous patch, but no {} here either.
@@ +898,4 @@
> {
> // With Proxies, we cannot garantee any property access as the proxy can
> // mask any property from the prototype chain.
> + if (!checkObj->isProxy())
At the start of function we do:
if (!checkObj || !checkObj->isNative())
return false;
Proxies are non-native, so you can turn this into an assert:
JS_ASSERT(!checkObj->isProxy());
Attachment #729155 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/202716eedf73
https://hg.mozilla.org/mozilla-central/rev/7538d3ff1e14
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in
before you can comment on or make changes to this bug.
Description
•