The critical loop for the v8-richards benchmark is in this function, the task scheduler: Scheduler.prototype.schedule = function () { this.currentTcb = this.list; while (this.currentTcb != null) { if (this.currentTcb.isHeldOrSuspended()) { this.currentTcb = this.currentTcb.link; } else { this.currentId = this.currentTcb.id; this.currentTcb = this.currentTcb.run(); } } }; Note all those .currentTcb accesses in there. None of those accesses are folded together, and there are type barriers on all but the one in the loop test (to watch for nulls produced afterwards). It is, however, ok to fold these together, as the only intervening function call which could modify this.currentTcb is the isHeldOrSuspended call, and that is inlined and does not write anywhere. If I rewrite this function to manually fold redundant currentTcb accesses: Scheduler.prototype.schedule = function () { this.currentTcb = this.list; var why = this.currentTcb; while (why != null) { if (why.isHeldOrSuspended()) { this.currentTcb = why.link; } else { this.currentId = why.id; this.currentTcb = why.run(); } why = this.currentTcb; } }; Combined with the patch in bug 824249, this improves the benchmark score from 10006 (trunk) to 13476. v8 is 12919, and is unaffected by this modification (i.e. it is already doing this folding). I'm not sure yet what changes are necessary to ensure we can do this folding automatically, but may cut across several areas (GVN, alias analysis, type barrier elimination) so filing now.

Bug 824275 fixes the redundant loads of this.currentTcb, but there are still type barriers that execute for each of the folded loads. The attached patch gets rid of these by determining that the barrier is redundant given the earlier non-barriered access and null/undefined test on the result it produces. This analysis is pretty specialized and is really just the starting point for optimizing away null/undefined checks. Still, this is a good start with low overhead and does pretty much all that's needed for v8-richards. Combined with bug 824257 (which this needs) and bug 824249 I get scores on the benchmark of around 13800.

Fix dumb mistake (looked for a not-null/undefined test, but didn't check what operand it was actually testing), add a test for the problem.

Comment on attachment 695262 [details] [diff] [review] eliminate redundant type barriers Review of attachment 695262 [details] [diff] [review]: ----------------------------------------------------------------- This optimization is indeed pretty specialized, but considering the possible wins it seems worth it. The issue below may slightly complicate things though. ::: js/src/ion/IonAnalysis.cpp @@ +1239,5 @@ > + break; > + case JSOP_NE: > + case JSOP_STRICTNE: > + if (direction == TRUE_BRANCH) > + ensuresNotNullOrUndefined = true; If we have if (x.y != null) x.y; Inside the "if", x.y must be non-null and non-undefined. However, if it's a strict comparison, x.y can still be undefined.

Oops, good point. Attached patch corrects for this and adds a test which fails on the earlier patch.

Nice. It looks like this was about a 13% win on richards on AWFY. Oddly enough, that happens to cancel out a regression that happened a few weeks ago. Do we know what that regression was yet?

(In reply to David Mandelin [:dmandelin] from comment #7) > Nice. It looks like this was about a 13% win on richards on AWFY. Oddly > enough, that happens to cancel out a regression that happened a few weeks > ago. Do we know what that regression was yet? I think it's bug 821040.