[clang] Fix quadratic slowdown in AST matcher parent map generation (PR #87824)

via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 9 08:45:23 PDT 2024


https://github.com/higher-performance updated https://github.com/llvm/llvm-project/pull/87824

>From 5e2b6b996a938129c087c5538155fb1b39e59ce8 Mon Sep 17 00:00:00 2001
From: higher-performance
 <113926381+higher-performance at users.noreply.github.com>
Date: Fri, 5 Apr 2024 15:36:05 -0400
Subject: [PATCH 1/2] Fix quadratic slowdown in AST matcher parent map
 generation

Avoids the need to linearly re-scan all seen parent nodes to check for duplicates, which previously caused a slowdown for ancestry checks in Clang AST matchers.

Fixes https://github.com/llvm/llvm-project/issues/86881.
---
 clang/lib/AST/ParentMapContext.cpp | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/clang/lib/AST/ParentMapContext.cpp b/clang/lib/AST/ParentMapContext.cpp
index 21cfd5b1de6e9d..3369df38754485 100644
--- a/clang/lib/AST/ParentMapContext.cpp
+++ b/clang/lib/AST/ParentMapContext.cpp
@@ -61,7 +61,28 @@ class ParentMapContext::ParentMap {
   template <typename, typename...> friend struct ::MatchParents;
 
   /// Contains parents of a node.
-  using ParentVector = llvm::SmallVector<DynTypedNode, 2>;
+  class ParentVector {
+  public:
+    ParentVector() = default;
+    explicit ParentVector(size_t n, const DynTypedNode &value) {
+      Items.reserve(n);
+      for (; n > 0; --n) {
+        push_back(value);
+      }
+    }
+    bool contains(const DynTypedNode &value) {
+      return Seen.contains(value);
+    }
+    void push_back(const DynTypedNode &value) {
+      if (!value.getMemoizationData() || Seen.insert(value).second) {
+        Items.push_back(value);
+      }
+    }
+    llvm::ArrayRef<DynTypedNode> view() const { return Items; }
+  private:
+    llvm::SmallVector<DynTypedNode, 2> Items;
+    llvm::SmallDenseSet<DynTypedNode, 2> Seen;
+  };
 
   /// Maps from a node to its parents. This is used for nodes that have
   /// pointer identity only, which are more common and we can save space by
@@ -99,7 +120,7 @@ class ParentMapContext::ParentMap {
       return llvm::ArrayRef<DynTypedNode>();
     }
     if (const auto *V = I->second.template dyn_cast<ParentVector *>()) {
-      return llvm::ArrayRef(*V);
+      return V->view();
     }
     return getSingleDynTypedNodeFromParentMap(I->second);
   }
@@ -252,7 +273,7 @@ class ParentMapContext::ParentMap {
       const auto *S = It->second.dyn_cast<const Stmt *>();
       if (!S) {
         if (auto *Vec = It->second.dyn_cast<ParentVector *>())
-          return llvm::ArrayRef(*Vec);
+          return Vec->view();
         return getSingleDynTypedNodeFromParentMap(It->second);
       }
       const auto *P = dyn_cast<Expr>(S);

>From ae3dacc99d53d86409384d50664c6804ccfdf195 Mon Sep 17 00:00:00 2001
From: higher-performance
 <113926381+higher-performance at users.noreply.github.com>
Date: Tue, 9 Apr 2024 11:45:17 -0400
Subject: [PATCH 2/2] Update clang/lib/AST/ParentMapContext.cpp

Co-authored-by: Aaron Ballman <aaron at aaronballman.com>
---
 clang/lib/AST/ParentMapContext.cpp | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/clang/lib/AST/ParentMapContext.cpp b/clang/lib/AST/ParentMapContext.cpp
index 3369df38754485..9723c0cfa83bbe 100644
--- a/clang/lib/AST/ParentMapContext.cpp
+++ b/clang/lib/AST/ParentMapContext.cpp
@@ -64,19 +64,17 @@ class ParentMapContext::ParentMap {
   class ParentVector {
   public:
     ParentVector() = default;
-    explicit ParentVector(size_t n, const DynTypedNode &value) {
-      Items.reserve(n);
-      for (; n > 0; --n) {
-        push_back(value);
-      }
+    explicit ParentVector(size_t N, const DynTypedNode &Value) {
+      Items.reserve(N);
+      for (; N > 0; --N)
+        push_back(Value);
     }
-    bool contains(const DynTypedNode &value) {
-      return Seen.contains(value);
+    bool contains(const DynTypedNode &Value) {
+      return Seen.contains(Value);
     }
-    void push_back(const DynTypedNode &value) {
-      if (!value.getMemoizationData() || Seen.insert(value).second) {
-        Items.push_back(value);
-      }
+    void push_back(const DynTypedNode &Value) {
+      if (!Value.getMemoizationData() || Seen.insert(Value).second)
+        Items.push_back(Value);
     }
     llvm::ArrayRef<DynTypedNode> view() const { return Items; }
   private:



More information about the cfe-commits mailing list