Closed Bug 1046176 Opened 5 years ago Closed 5 years ago

UnsafeSetReservedSlot post barrier always bails

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34
Tracking Status
firefox31 --- unaffected
firefox32 + fixed
firefox33 + fixed
firefox34 + fixed

People

(Reporter: jandem, Assigned: jandem)

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
Using callInfo.thisArg() instead of callInfo.getArg(0) means we'll always bail (thisArg() is undefined and MPostWriteBarrier expects an object).

With this patch, the for-of micro-benchmark below improves from 152 to 12 ms. This affects all for-of loops so we should fix it on branches as well.

function f(arr) {
    var t = new Date;
    for (var i=0; i<100000; i++) {
	for (var e of arr) {}
    }
    print(new Date - t);
}
f([1, 2, 3]);
Attachment #8464739 - Flags: review?(nicolas.b.pierron)
Attachment #8464739 - Flags: review?(nicolas.b.pierron) → review+
[Tracking Requested - why for this release]:

Pretty serious performance issue (see numbers in comment 0) with a trivial fix. GGC will be enabled in 32, so this only affects beta and later.
https://hg.mozilla.org/mozilla-central/rev/f578233e4d2c
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Comment on attachment 8464739 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Bug 928140, but it only affects versions with GGC (Firefox 32+) 
[User impact if declined]: Pretty serious performance issues in some cases
[Describe test coverage new/current, TBPL]: Patch is on m-c; trivial fix
[Risks and why]: Very low risk; one-liner
[String/UUID change made/needed]: None
Attachment #8464739 - Flags: approval-mozilla-beta?
Attachment #8464739 - Flags: approval-mozilla-aurora?
Attachment #8464739 - Flags: approval-mozilla-beta?
Attachment #8464739 - Flags: approval-mozilla-beta+
Attachment #8464739 - Flags: approval-mozilla-aurora?
Attachment #8464739 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.