Closed Bug 1171405 Opened 5 years ago Closed 5 years ago

Ion GetElement IC doesn't handle unboxed objects

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: jandem, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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)
(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.
Attached patch patchSplinter Review
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)
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+
https://hg.mozilla.org/mozilla-central/rev/7ccb13e479c4
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.