Skip to content

Commit 82d92bb

Browse files
perf: cache repeated tree walks to avoid O(N^2) in optimizeTerminatingTails
1 parent 4301eae commit 82d92bb

File tree

1 file changed

+38
-5
lines changed

1 file changed

+38
-5
lines changed

src/passes/CodeFolding.cpp

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ struct CodeFolding
299299
returnTails.clear();
300300
unoptimizables.clear();
301301
modifieds.clear();
302+
bodyCachePopulated = false;
302303
if (needEHFixups) {
303304
EHUtils::handleBlockNestedPops(func, *getModule());
304305
}
@@ -311,6 +312,41 @@ struct CodeFolding
311312
// inside that item
312313
bool canMove(const std::vector<Expression*>& items, Expression* outOf) {
313314
auto allTargets = BranchUtils::getBranchTargets(outOf);
315+
bool hasTry = false;
316+
bool hasTryTable = false;
317+
if (getModule()->features.hasExceptionHandling()) {
318+
hasTry = FindAll<Try>(outOf).has();
319+
hasTryTable = FindAll<TryTable>(outOf).has();
320+
}
321+
return canMoveImpl(items, allTargets, hasTry, hasTryTable);
322+
}
323+
324+
// Cached data for the function body, computed on demand to avoid repeated
325+
// O(N) tree walks in optimizeTerminatingTails.
326+
BranchUtils::NameSet bodyBranchTargets;
327+
bool bodyHasTry = false;
328+
bool bodyHasTryTable = false;
329+
bool bodyCachePopulated = false;
330+
331+
// Like canMove, but uses precomputed branch targets and Try/TryTable
332+
// presence. This avoids repeated O(N) tree walks when outOf is the
333+
// function body and canMove is called multiple times.
334+
bool canMoveWithCachedBodyInfo(const std::vector<Expression*>& items) {
335+
if (!bodyCachePopulated) {
336+
bodyBranchTargets = BranchUtils::getBranchTargets(getFunction()->body);
337+
if (getModule()->features.hasExceptionHandling()) {
338+
bodyHasTry = FindAll<Try>(getFunction()->body).has();
339+
bodyHasTryTable = FindAll<TryTable>(getFunction()->body).has();
340+
}
341+
bodyCachePopulated = true;
342+
}
343+
return canMoveImpl(items, bodyBranchTargets, bodyHasTry, bodyHasTryTable);
344+
}
345+
346+
bool canMoveImpl(const std::vector<Expression*>& items,
347+
const BranchUtils::NameSet& allTargets,
348+
bool hasTry,
349+
bool hasTryTable) {
314350
for (auto* item : items) {
315351
auto exiting = BranchUtils::getExitingBranches(item);
316352
std::vector<Name> intersection;
@@ -341,9 +377,7 @@ struct CodeFolding
341377
// conservative approximation because there can be cases that
342378
// 'try'/'try_table' is within the expression that may throw so it is
343379
// safe to take the expression out.
344-
// TODO: optimize this check to avoid two FindAlls.
345-
if (effects.throws() &&
346-
(FindAll<Try>(outOf).has() || FindAll<TryTable>(outOf).has())) {
380+
if (effects.throws() && (hasTry || hasTryTable)) {
347381
return false;
348382
}
349383
}
@@ -610,8 +644,7 @@ struct CodeFolding
610644
cost += WORTH_ADDING_BLOCK_TO_REMOVE_THIS_MUCH;
611645
// if we cannot merge to the end, then we definitely need 2 blocks,
612646
// and a branch
613-
// TODO: efficiency, entire body
614-
if (!canMove(items, getFunction()->body)) {
647+
if (!canMoveWithCachedBodyInfo(items)) {
615648
cost += 1 + WORTH_ADDING_BLOCK_TO_REMOVE_THIS_MUCH;
616649
// TODO: to do this, we need to maintain a map of element=>parent,
617650
// so that we can insert the new blocks in the right place

0 commit comments

Comments
 (0)