r177353 - [analyzer] Do part of the work to find shortest bug paths up front.

Jordan Rose jordan_rose at apple.com
Mon Mar 18 16:34:38 PDT 2013


Author: jrose
Date: Mon Mar 18 18:34:37 2013
New Revision: 177353

URL: http://llvm.org/viewvc/llvm-project?rev=177353&view=rev
Log:
[analyzer] Do part of the work to find shortest bug paths up front.

Splitting the graph trimming and the path-finding (r177216) already
recovered quite a bit of performance lost to increased suppression.
We can still do better by also performing the reverse BFS up front
(needed for shortest-path-finding) and only walking the shortest path
for each report. This does mean we have to walk back up the path and
invalidate all the BFS numbers if the report turns out to be invalid,
but it's probably still faster than redoing the full BFS every time.

More performance work for <rdar://problem/13433687>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=177353&r1=177352&r2=177353&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Mon Mar 18 18:34:37 2013
@@ -1888,86 +1888,109 @@ public:
 class TrimmedGraph {
   InterExplodedGraphMap ForwardMap;
   InterExplodedGraphMap InverseMap;
+
+  typedef llvm::DenseMap<const ExplodedNode *, unsigned> PriorityMapTy;
+  PriorityMapTy PriorityMap;
+
   OwningPtr<ExplodedGraph> G;
+  const ExplodedNode *Root;
 public:
-  /// 
   TrimmedGraph(const ExplodedGraph *OriginalGraph,
-               ArrayRef<const ExplodedNode *> Nodes) {
-    // The trimmed graph is created in the body of the constructor to ensure
-    // that the DenseMaps have been initialized already.
-    G.reset(OriginalGraph->trim(Nodes, &ForwardMap, &InverseMap));
-  }
+               ArrayRef<const ExplodedNode *> Nodes);
 
   void createBestReportGraph(ArrayRef<const ExplodedNode *> Nodes,
                              ReportGraph &GraphWrapper) const;
+
+  void removeErrorNode(const ExplodedNode *Node);
 };
 
 }
 
-
-void TrimmedGraph::createBestReportGraph(ArrayRef<const ExplodedNode *> Nodes,
-                                         ReportGraph &GraphWrapper) const {
-  assert(!GraphWrapper.Graph && "ReportGraph is already in use");
-  assert(GraphWrapper.BackMap.empty() && "ReportGraph is already in use");
+TrimmedGraph::TrimmedGraph(const ExplodedGraph *OriginalGraph,
+                           ArrayRef<const ExplodedNode *> Nodes) {
+  // The trimmed graph is created in the body of the constructor to ensure
+  // that the DenseMaps have been initialized already.
+  G.reset(OriginalGraph->trim(Nodes, &ForwardMap, &InverseMap));
 
   // Find the (first) error node in the trimmed graph.  We just need to consult
   // the node map which maps from nodes in the original graph to nodes
   // in the new graph.
-  std::queue<const ExplodedNode *> WS;
+  std::queue<std::pair<const ExplodedNode *, unsigned> > WS;
   typedef llvm::SmallDenseMap<const ExplodedNode *, size_t, 32> IndexMapTy;
-  IndexMapTy IndexMap;
+  IndexMapTy IndexMap(llvm::NextPowerOf2(Nodes.size() + 1));
 
   for (unsigned i = 0, count = Nodes.size(); i < count; ++i) {
     const ExplodedNode *OriginalNode = Nodes[i];
     if (const ExplodedNode *N = ForwardMap.lookup(OriginalNode)) {
-      WS.push(N);
+      WS.push(std::make_pair(N, 0));
       IndexMap[OriginalNode] = i;
     }
   }
 
   assert(!WS.empty() && "No error node found in the trimmed graph.");
 
-  // Create a new graph with a single path.  This is the graph
-  // that will be returned to the caller.
-  ExplodedGraph *GNew = new ExplodedGraph();
-  GraphWrapper.Graph.reset(GNew);
-
-  // Sometimes the trimmed graph can contain a cycle.  Perform a reverse BFS
-  // to the root node, and then construct a new graph that contains only
-  // a single path.
-  llvm::DenseMap<const ExplodedNode *, unsigned> Visited;
-
-  unsigned cnt = 0;
-  const ExplodedNode *Root = 0;
-
+  // Perform a reverse BFS to find all the shortest paths.
+  Root = 0;
   while (!WS.empty()) {
-    const ExplodedNode *Node = WS.front();
+    const ExplodedNode *Node;
+    unsigned Priority;
+    llvm::tie(Node, Priority) = WS.front();
     WS.pop();
 
-    if (Visited.find(Node) != Visited.end())
+    PriorityMapTy::iterator I = PriorityMap.find(Node);
+    if (I != PriorityMap.end()) {
+      assert(I->second <= Priority);
       continue;
+    }
 
-    Visited[Node] = cnt++;
+    PriorityMap[Node] = Priority;
 
-    if (Node->pred_empty()) {
+    if (Node->pred_empty())
       Root = Node;
-      break;
-    }
 
-    for (ExplodedNode::const_pred_iterator I=Node->pred_begin(),
-         E=Node->pred_end(); I!=E; ++I)
-      WS.push(*I);
+    for (ExplodedNode::const_pred_iterator I = Node->pred_begin(),
+                                           E = Node->pred_end();
+         I != E; ++I)
+      WS.push(std::make_pair(*I, Priority + 1));
   }
-
+  
   assert(Root);
+}
+
+void TrimmedGraph::createBestReportGraph(ArrayRef<const ExplodedNode *> Nodes,
+                                         ReportGraph &GraphWrapper) const {
+  assert(!GraphWrapper.Graph && "ReportGraph is already in use");
+  assert(GraphWrapper.BackMap.empty() && "ReportGraph is already in use");
+
+  // Find the (first) error node in the trimmed graph.  We just need to consult
+  // the node map which maps from nodes in the original graph to nodes
+  // in the new graph.
+  std::queue<std::pair<const ExplodedNode *, unsigned> > WS;
+  typedef llvm::SmallDenseMap<const ExplodedNode *, size_t, 32> IndexMapTy;
+  IndexMapTy IndexMap(llvm::NextPowerOf2(Nodes.size() + 1));
+
+  for (unsigned i = 0, count = Nodes.size(); i < count; ++i) {
+    const ExplodedNode *OriginalNode = Nodes[i];
+    if (const ExplodedNode *N = ForwardMap.lookup(OriginalNode)) {
+      WS.push(std::make_pair(N, 0));
+      IndexMap[OriginalNode] = i;
+    }
+  }
+
+  assert(!WS.empty() && "No error node found in the trimmed graph.");
+
+  // Create a new graph with a single path.  This is the graph
+  // that will be returned to the caller.
+  ExplodedGraph *GNew = new ExplodedGraph();
+  GraphWrapper.Graph.reset(GNew);
 
   // Now walk from the root down the BFS path, always taking the successor
   // with the lowest number.
   ExplodedNode *Last = 0;
   for ( const ExplodedNode *N = Root ;;) {
     // Lookup the number associated with the current node.
-    llvm::DenseMap<const ExplodedNode *,unsigned>::iterator I = Visited.find(N);
-    assert(I != Visited.end());
+    PriorityMapTy::const_iterator I = PriorityMap.find(N);
+    assert(I != PriorityMap.end());
 
     // Create the equivalent node in the new graph with the same state
     // and location.
@@ -1998,9 +2021,9 @@ void TrimmedGraph::createBestReportGraph
     for (ExplodedNode::const_succ_iterator SI = N->succ_begin(),
                                            SE = N->succ_end();
          SI != SE; ++SI) {
-      I = Visited.find(*SI);
+      I = PriorityMap.find(*SI);
 
-      if (I == Visited.end())
+      if (I == PriorityMap.end())
         continue;
 
       if (I->second < MinVal) {
@@ -2013,6 +2036,54 @@ void TrimmedGraph::createBestReportGraph
   }
 }
 
+void TrimmedGraph::removeErrorNode(const ExplodedNode *ErrorNode) {
+  ErrorNode = ForwardMap[ErrorNode];
+  assert(ErrorNode && "not an error node");
+
+  PriorityMapTy::iterator PriorityEntry = PriorityMap.find(ErrorNode);
+  assert(PriorityEntry != PriorityMap.end() && "error node already removed");
+  PriorityMap.erase(PriorityEntry);
+
+  std::queue<const ExplodedNode *> WS;
+  for (ExplodedNode::const_pred_iterator PI = ErrorNode->pred_begin(),
+                                         PE = ErrorNode->pred_end();
+       PI != PE; ++PI) {
+    assert(PriorityMap.find(*PI) != PriorityMap.end());
+    WS.push(*PI);
+  }
+
+  // Update all nodes possibly affected by this change.
+  while (!WS.empty()) {
+    const ExplodedNode *N = WS.front();
+    WS.pop();
+
+    unsigned MinPriority = -1U;
+    for (ExplodedNode::const_succ_iterator SI = N->succ_begin(),
+                                           SE = N->succ_end();
+         SI != SE; ++SI) {
+      PriorityMapTy::iterator SuccEntry = PriorityMap.find(*SI);
+      if (SuccEntry == PriorityMap.end())
+        continue;
+      MinPriority = std::min(SuccEntry->second, MinPriority);
+    }
+
+    if (MinPriority == -1U)
+      PriorityMap.erase(N);
+    else if (PriorityMap[N] == MinPriority + 1)
+      continue;
+    else
+      PriorityMap[N] = MinPriority + 1;
+
+    for (ExplodedNode::const_pred_iterator PI = N->pred_begin(),
+                                           PE = N->pred_end();
+         PI != PE; ++PI) {
+      assert(PriorityMap.find(*PI) != PriorityMap.end());
+      WS.push(*PI);
+    }
+  }
+}
+
+
 /// CompactPathDiagnostic - This function postprocesses a PathDiagnostic object
 ///  and collapses PathDiagosticPieces that are expanded by macros.
 static void CompactPathDiagnostic(PathPieces &path, const SourceManager& SM) {
@@ -2215,8 +2286,10 @@ bool GRBugReporter::generatePathDiagnost
       finalReportConfigToken = R->getConfigurationChangeToken();
     } while (finalReportConfigToken != origReportConfigToken);
 
-    if (!R->isValid())
+    if (!R->isValid()) {
+      TrimG.removeErrorNode(R->getErrorNode());
       continue;
+    }
 
     // Finally, prune the diagnostic path of uninteresting stuff.
     if (!PD.path.empty()) {





More information about the cfe-commits mailing list