Skip to content

Commit a1924cf

Browse files
perf: cache repeated tree walks to avoid O(N^2) in optimizeTerminatingTails
1 parent c23ed20 commit a1924cf

File tree

1 file changed

+36
-5
lines changed

1 file changed

+36
-5
lines changed

src/passes/CodeFolding.cpp

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,12 @@ struct CodeFolding
140140
// walk to avoid O(N^2) repeated tree traversals on nested blocks.
141141
std::unordered_map<Expression*, bool> exitingBranchCache_;
142142

143+
// Cached data for the function body, computed once per
144+
// optimizeTerminatingTails call to avoid repeated O(N) tree walks.
145+
BranchUtils::NameSet bodyBranchTargets_;
146+
bool bodyHasTry_ = false;
147+
bool bodyHasTryTable_ = false;
148+
143149
// walking
144150

145151
void visitExpression(Expression* curr) {
@@ -368,6 +374,27 @@ struct CodeFolding
368374
// inside that item
369375
bool canMove(const std::vector<Expression*>& items, Expression* outOf) {
370376
auto allTargets = BranchUtils::getBranchTargets(outOf);
377+
bool hasTry = false;
378+
bool hasTryTable = false;
379+
if (getModule()->features.hasExceptionHandling()) {
380+
hasTry = FindAll<Try>(outOf).has();
381+
hasTryTable = FindAll<TryTable>(outOf).has();
382+
}
383+
return canMoveImpl(items, allTargets, hasTry, hasTryTable);
384+
}
385+
386+
// Like canMove, but uses precomputed branch targets and Try/TryTable
387+
// presence. This avoids repeated O(N) tree walks when outOf is the
388+
// function body and canMove is called multiple times.
389+
bool canMoveWithCachedBodyInfo(const std::vector<Expression*>& items) {
390+
return canMoveImpl(
391+
items, bodyBranchTargets_, bodyHasTry_, bodyHasTryTable_);
392+
}
393+
394+
bool canMoveImpl(const std::vector<Expression*>& items,
395+
const BranchUtils::NameSet& allTargets,
396+
bool hasTry,
397+
bool hasTryTable) {
371398
for (auto* item : items) {
372399
auto exiting = BranchUtils::getExitingBranches(item);
373400
std::vector<Name> intersection;
@@ -398,9 +425,7 @@ struct CodeFolding
398425
// conservative approximation because there can be cases that
399426
// 'try'/'try_table' is within the expression that may throw so it is
400427
// safe to take the expression out.
401-
// TODO: optimize this check to avoid two FindAlls.
402-
if (effects.throws() &&
403-
(FindAll<Try>(outOf).has() || FindAll<TryTable>(outOf).has())) {
428+
if (effects.throws() && (hasTry || hasTryTable)) {
404429
return false;
405430
}
406431
}
@@ -610,6 +635,13 @@ struct CodeFolding
610635
// exitingBranchCache_ lookups are O(1).
611636
if (num == 0) {
612637
populateExitingBranchCache(getFunction()->body);
638+
// Cache branch targets and Try/TryTable presence for the function body
639+
// so that canMove doesn't re-walk the entire body on every call.
640+
bodyBranchTargets_ = BranchUtils::getBranchTargets(getFunction()->body);
641+
if (getModule()->features.hasExceptionHandling()) {
642+
bodyHasTry_ = FindAll<Try>(getFunction()->body).has();
643+
bodyHasTryTable_ = FindAll<TryTable>(getFunction()->body).has();
644+
}
613645
}
614646
// remove things that are untoward and cannot be optimized
615647
tails.erase(
@@ -672,8 +704,7 @@ struct CodeFolding
672704
cost += WORTH_ADDING_BLOCK_TO_REMOVE_THIS_MUCH;
673705
// if we cannot merge to the end, then we definitely need 2 blocks,
674706
// and a branch
675-
// TODO: efficiency, entire body
676-
if (!canMove(items, getFunction()->body)) {
707+
if (!canMoveWithCachedBodyInfo(items)) {
677708
cost += 1 + WORTH_ADDING_BLOCK_TO_REMOVE_THIS_MUCH;
678709
// TODO: to do this, we need to maintain a map of element=>parent,
679710
// so that we can insert the new blocks in the right place

0 commit comments

Comments
 (0)