Closed
Bug 1171405
Opened 10 years ago
Closed 9 years ago
Ion GetElement IC doesn't handle unboxed objects
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: jandem, Assigned: bhackett1024)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
Noticed this issue on the Flux benchmark in bug 1169501. It spends some time in GetElementIC::update, we don't attach a stub there because GetElementIC::canAttachGetProp returns false.
For the testcase below I get:
js --no-unboxed-objects: 40 ms
js --no-ion : 176 ms
js : 3386 ms
Based on these numbers, it looks like Baseline handles this case.
function O() {
this.x = 1;
this.y = 2;
}
function f() {
var arr = [];
for (var i=0; i<64; i++)
arr.push(new O);
var res;
for (var i=0; i<10000000; i++) {
var o = arr[i % 64];
var p = (i & 1) ? "x" : "y";
var res = o[p];
}
return res;
}
var t = new Date;
f();
print(new Date - t);
Flags: needinfo?(bhackett1024)
Reporter | ||
Comment 1•10 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #0)
> js --no-unboxed-objects: 40 ms
> js --no-ion : 176 ms
> js : 3386 ms
Oops, the last one is a bit too pessimistic; I forgot I added some logging there. It should be more like 445 ms, still > 10x slower.
Assignee | ||
Comment 2•10 years ago
|
||
Actually, there are neither ion nor baseline IC stubs for this case. Using --no-ion disables the new script properties analysis, which is needed to make the 'new O()' objects unboxed. The attached patch adds ion and baseline IC stubs for GETELEM on both own and expando properties in unboxed objects.
Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)
Attachment #8617491 -
Flags: review?(jdemooij)
Reporter | ||
Comment 3•9 years ago
|
||
Comment on attachment 8617491 [details] [diff] [review]
patch
Review of attachment 8617491 [details] [diff] [review]:
-----------------------------------------------------------------
Sorry for the delay.
Attachment #8617491 -
Flags: review?(jdemooij) → review+
Comment 5•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•