Bug 1297804 - part 4 - avoid array bounds checks in DestroySubtree loops; r=billm
authorNathan Froyd <froydnj@gmail.com>
Fri, 02 Sep 2016 16:14:28 -0400
changeset 312415 1b874fea3cbcc1895dc5e97bd31582c920b6909f
parent 312414 af44291ba592257c4ced0a91ba77ca448bb6bb42
child 312416 c4fb41365df3259c6d43b6b897c4aebfc93a08d9
push id20447
push userkwierso@gmail.com
push dateFri, 02 Sep 2016 20:36:44 +0000
treeherderfx-team@969397f22187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1297804
milestone51.0a1
Bug 1297804 - part 4 - avoid array bounds checks in DestroySubtree loops; r=billm The IPDL compiler generates code like this in DestroySubtree methods: // Recursively shutting down PAPZ kids nsTArray<PAPZChild*> kids((mManagedPAPZChild).Count()); // Accumulate kids into a stable structure to iterate over ManagedPAPZChild(kids); for (uint32_t i = 0; (i) < ((kids).Length()); (++(i))) { // Guarding against a child removing a sibling from the list during the iteration. if ((mManagedPAPZChild).Contains(kids[i])) { (kids[i])->DestroySubtree(subtreewhy); } } There are three problems with this code: 1) We initialize |kids| with a capacity, which is wasted work; ManagedPAPZChild() is going to resize the array for us anyway. 2) We're constantly reading from |kids.Length()| in the loop, when we only really need to read from it once. 3) We're repeatedly accessing kids[i], and doing needless bounds checks. Let's fix those three problems.
ipc/ipdl/ipdl/lower.py
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -3455,38 +3455,36 @@ class _GenerateProtocolActorCode(ipdl.as
                             ExprBinary(whyvar, '==',
                                        _DestroyReason.FailedConstructor)),
                         _DestroyReason.AncestorDeletion, whyvar)),
                 Whitespace.NL
             ])
 
         for managed in ptype.manages:
             managedVar = p.managedVar(managed, self.side)
-
-            foreachdestroy = StmtFor(
-                init=Param(Type.UINT32, ivar.name, ExprLiteral.ZERO),
-                cond=ExprBinary(ivar, '<', _callCxxArrayLength(kidsvar)),
-                update=ExprPrefixUnop(ivar, '++'))
+            lenvar = ExprVar('len')
+            kidvar = ExprVar('kid')
+
+            foreachdestroy = StmtRangedFor(kidvar, kidsvar)
 
             foreachdestroy.addstmt(
                 Whitespace('// Guarding against a child removing a sibling from the list during the iteration.\n', indent=1))
-            ifhas = StmtIf(_callHasManagedActor(managedVar, ithkid))
+            ifhas = StmtIf(_callHasManagedActor(managedVar, kidvar))
             ifhas.addifstmt(StmtExpr(ExprCall(
-                ExprSelect(ithkid, '->', destroysubtreevar.name),
+                ExprSelect(kidvar, '->', destroysubtreevar.name),
                 args=[ subtreewhyvar ])))
             foreachdestroy.addstmt(ifhas)
 
             block = StmtBlock()
             block.addstmts([
                 Whitespace(
                     '// Recursively shutting down %s kids\n'% (managed.name()),
                     indent=1),
                 StmtDecl(
-                    Decl(_cxxArrayType(p.managedCxxType(managed, self.side)), kidsvar.name),
-                    initargs=[ ExprCall(ExprSelect(managedVar, '.', 'Count')) ]),
+                    Decl(_cxxArrayType(p.managedCxxType(managed, self.side)), kidsvar.name)),
                 Whitespace(
                     '// Accumulate kids into a stable structure to iterate over\n',
                     indent=1),
                 StmtExpr(ExprCall(p.managedMethod(managed, self.side),
                                   args=[ kidsvar ])),
                 foreachdestroy,
             ])
             destroysubtree.addstmt(block)