hokein created this revision.
hokein added reviewers: klimek, sammccall.
Herald added a project: clang.
D80025 <https://reviews.llvm.org/D80025> introduced a performance regression:
in some cases, it makes
clang-tidy readability-container-size-empty ~80x slower (running on an internal
huge TU, before that patch 12s vs after 950s).
after this patch, we go back to 12s.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D82771
Files:
clang/lib/ASTMatchers/ASTMatchFinder.cpp
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -45,7 +45,9 @@
enum class MatchType {
Ancestors,
- Descendants
+
+ Descendants,
+ Child,
};
// We use memoization to avoid running the same matcher on the same
@@ -452,8 +454,7 @@
BoundNodesTreeBuilder *Builder, int MaxDepth,
TraversalKind Traversal, BindKind Bind) {
// For AST-nodes that don't have an identity, we can't memoize.
- // When doing a single-level match, we don't need to memoize
- if (!Node.getMemoizationData() || !Builder->isComparable() || MaxDepth ==
1)
+ if (!Node.getMemoizationData() || !Builder->isComparable())
return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
Bind);
@@ -463,7 +464,8 @@
// Note that we key on the bindings *before* the match.
Key.BoundNodes = *Builder;
Key.Traversal = Ctx.getParentMapContext().getTraversalKind();
- Key.Type = MatchType::Descendants;
+ // Memorize result even doing a single-level match, it might be expensive.
+ Key.Type = MaxDepth == 1 ? MatchType::Child : MatchType::Descendants;
MemoizationMap::iterator I = ResultCache.find(Key);
if (I != ResultCache.end()) {
*Builder = I->second.Nodes;
@@ -700,8 +702,10 @@
BoundNodesTreeBuilder *Builder,
AncestorMatchMode MatchMode) {
// For AST-nodes that don't have an identity, we can't memoize.
- // When doing a single-level match, we don't need to memoize
- if (!Builder->isComparable() || MatchMode ==
AncestorMatchMode::AMM_ParentOnly)
+ // When doing a single-level match, we don't need to memoize because
+ // ParentMap (in ASTContext) already memorizes the result.
+ if (!Builder->isComparable() ||
+ MatchMode == AncestorMatchMode::AMM_ParentOnly)
return matchesAncestorOfRecursively(Node, Ctx, Matcher, Builder,
MatchMode);
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -45,7 +45,9 @@
enum class MatchType {
Ancestors,
- Descendants
+
+ Descendants,
+ Child,
};
// We use memoization to avoid running the same matcher on the same
@@ -452,8 +454,7 @@
BoundNodesTreeBuilder *Builder, int MaxDepth,
TraversalKind Traversal, BindKind Bind) {
// For AST-nodes that don't have an identity, we can't memoize.
- // When doing a single-level match, we don't need to memoize
- if (!Node.getMemoizationData() || !Builder->isComparable() || MaxDepth == 1)
+ if (!Node.getMemoizationData() || !Builder->isComparable())
return matchesRecursively(Node, Matcher, Builder, MaxDepth, Traversal,
Bind);
@@ -463,7 +464,8 @@
// Note that we key on the bindings *before* the match.
Key.BoundNodes = *Builder;
Key.Traversal = Ctx.getParentMapContext().getTraversalKind();
- Key.Type = MatchType::Descendants;
+ // Memorize result even doing a single-level match, it might be expensive.
+ Key.Type = MaxDepth == 1 ? MatchType::Child : MatchType::Descendants;
MemoizationMap::iterator I = ResultCache.find(Key);
if (I != ResultCache.end()) {
*Builder = I->second.Nodes;
@@ -700,8 +702,10 @@
BoundNodesTreeBuilder *Builder,
AncestorMatchMode MatchMode) {
// For AST-nodes that don't have an identity, we can't memoize.
- // When doing a single-level match, we don't need to memoize
- if (!Builder->isComparable() || MatchMode == AncestorMatchMode::AMM_ParentOnly)
+ // When doing a single-level match, we don't need to memoize because
+ // ParentMap (in ASTContext) already memorizes the result.
+ if (!Builder->isComparable() ||
+ MatchMode == AncestorMatchMode::AMM_ParentOnly)
return matchesAncestorOfRecursively(Node, Ctx, Matcher, Builder,
MatchMode);
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
