Replace the type variable -> graph node dense map with an embedded pointer.

Provides a 4% speedup type-checking the standard library. This
optimization brings the global constraint graph within 2% of the prior
solution, and prevents us from creating multiple constraint graphs per
constraint system, so flip the switch to always use the global
constraint graph.


Swift SVN r11003

lib/Sema/ConstraintGraph.cpp

@@ -33,9 +33,10 @@ ConstraintGraph::ConstraintGraph(ConstraintSystem &cs) : CS(cs) { }
 
 ConstraintGraph::~ConstraintGraph() {
   assert(Changes.empty() && "Scope stack corrupted");
-
+  for (unsigned i = 0, n = TypeVariables.size(); i != n; ++i) {
-  for (auto node : Nodes) {
+    auto &impl = TypeVariables[i]->getImpl();
-    delete node.second.NodePtr;
+    delete impl.getGraphNode();
+    impl.setGraphNode(0);
   }
 }
 
@@ -107,16 +108,19 @@ gatherReferencedTypeVars(ConstraintSystem &cs,
 std::pair<ConstraintGraphNode &, unsigned>
 ConstraintGraph::lookupNode(TypeVariableType *typeVar) {
   // Check whether we've already created a node for this type variable.
-  auto known = Nodes.find(typeVar);
+  auto &impl = typeVar->getImpl();
-  if (known != Nodes.end()) {
+  if (auto nodePtr = impl.getGraphNode()) {
-    assert(known->second.NodePtr && "Missing node pointer?");
+    assert(impl.getGraphIndex() < TypeVariables.size() && "Out-of-bounds index");
-    return { *known->second.NodePtr, known->second.Index };
+    assert(TypeVariables[impl.getGraphIndex()] == typeVar && 
+           "Type variable mismatch");
+    return { *nodePtr, impl.getGraphIndex() };
   }
 
   // Allocate the new node.
-  StoredNode &stored = Nodes[typeVar];
+  auto nodePtr = new ConstraintGraphNode(typeVar);
-  stored.NodePtr = new ConstraintGraphNode(typeVar);
+  unsigned index = TypeVariables.size();
-  stored.Index = TypeVariables.size();
+  impl.setGraphNode(nodePtr);
+  impl.setGraphIndex(index);
 
   // Record this type variable.
   TypeVariables.push_back(typeVar);
@@ -135,7 +139,7 @@ ConstraintGraph::lookupNode(TypeVariableType *typeVar) {
     bindTypeVariable(typeVar, fixed);
   }
 
-  return { *stored.NodePtr, stored.Index };
+  return { *nodePtr, index };
 }
 
 ArrayRef<TypeVariableType *> ConstraintGraphNode::getEquivalenceClass() const{
@@ -374,14 +378,11 @@ void ConstraintGraph::Change::undo(ConstraintGraph &cg) {
 #pragma mark Graph mutation
 
 void ConstraintGraph::removeNode(TypeVariableType *typeVar) {
-  // Find the node to remove.
-  auto pos = Nodes.find(typeVar);
-  assert(pos != Nodes.end() && "No node for this type variable");
-  
   // Remove this node.
-  unsigned index = pos->second.Index;
+  auto &impl = typeVar->getImpl();
-  delete pos->second.NodePtr;
+  unsigned index = impl.getGraphIndex();
-  Nodes.erase(pos);
+  delete impl.getGraphNode();
+  impl.setGraphNode(0);
 
   // Remove this type variable from the list.
   unsigned lastIndex = TypeVariables.size()-1;
@@ -921,18 +922,18 @@ void ConstraintGraph::verify() {
   }
 
   // Verify that our type variable map/vector are in sync.
-  requireSameValue(TypeVariables.size(), Nodes.size(),
+  for (unsigned i = 0, n = TypeVariables.size(); i != n; ++i) {
-                   "type variables vector and node map have different sizes");
+    auto typeVar = TypeVariables[i];
-  for (auto node : Nodes) {
+    auto &impl = typeVar->getImpl();
-    require(node.second.Index < TypeVariables.size(),
+    requireSameValue(impl.getGraphIndex(), i, "wrong graph node index");
-            "out of bounds node index");
+    require(impl.getGraphNode(), "null graph node");
-    requireSameValue(node.first, TypeVariables[node.second.Index],
-                     "node map provides wrong index into type variable vector");
   }
 
   // Verify consistency of all of the nodes in the graph.
-  for (auto node : Nodes) {
+  for (unsigned i = 0, n = TypeVariables.size(); i != n; ++i) {
-    node.second.NodePtr->verify(*this);
+    auto typeVar = TypeVariables[i];
+    auto &impl = typeVar->getImpl();
+    impl.getGraphNode()->verify(*this);
   }
 
   // Collect all of the constraints known to the constraint graph.
@@ -964,8 +965,8 @@ void ConstraintGraph::verify() {
     // Make sure each of the type variables referenced knows about this
     // constraint.
     for (auto typeVar : referencedTypeVars) {
-      auto nodePos = Nodes.find(typeVar);
+      auto nodePtr = typeVar->getImpl().getGraphNode();
-      requireWithContext(nodePos != Nodes.end(),
+      requireWithContext(nodePtr,
                          "type variable in constraint not known",
                          [&] {
                            llvm::dbgs() << "type variable = ";
@@ -975,7 +976,7 @@ void ConstraintGraph::verify() {
                            llvm::dbgs() << "\n";
                          });
 
-      auto &node = *nodePos->second.NodePtr;
+      auto &node = *nodePtr;
       auto constraintPos = node.ConstraintIndex.find(&constraint);
       requireWithContext(constraintPos != node.ConstraintIndex.end(),
                          "type variable doesn't know about constraint",

lib/Sema/ConstraintGraph.h

@@ -275,22 +275,6 @@ private:
   /// The type variables in this graph, in stable order.
   SmallVector<TypeVariableType *, 4> TypeVariables;
 
-  /// A stored node within the node mapping, containing both the node pointer
-  /// and the
-  struct StoredNode {
-    /// \brief The node itself, stored as a pointer so we can efficiently
-    /// copy/move \c StoredNodes.
-    ConstraintGraphNode *NodePtr;
-
-    /// \brief The index in the \c TypeVariables vector where the corresponding
-    /// type variable is stored.
-    unsigned Index;
-  };
-
-  /// A mapping from the type variables in the graph to their corresponding
-  /// nodes along with the index
-  llvm::DenseMap<TypeVariableType *, StoredNode> Nodes;
-
   /// The kind of change made to the graph.
   enum class ChangeKind {
     /// Added a type variable.

lib/Sema/ConstraintSystem.h

@@ -47,6 +47,7 @@ class Expr;
 namespace constraints {
 
 class ConstraintGraph;
+class ConstraintGraphNode;
 class ConstraintSystem;
 
 } // end namespace constraints
@@ -113,6 +114,13 @@ class TypeVariableType::Implementation {
   /// type is bound.
   llvm::PointerUnion<TypeVariableType *, TypeBase *> ParentOrFixed;
 
+  /// The corresponding node in the constraint graph.
+  constraints::ConstraintGraphNode *GraphNode = nullptr;
+
+  ///  Index into the list of type variables, as used by the
+  ///  constraint graph.
+  unsigned GraphIndex;
+
   friend class constraints::SavedTypeVariableBinding;
 
 public:
@@ -143,6 +151,23 @@ public:
     return reinterpret_cast<const TypeVariableType *>(this) - 1;
   }
 
+  /// Retrieve the corresponding node in the constraint graph.
+  constraints::ConstraintGraphNode *getGraphNode() const { return GraphNode; }
+
+  /// Set the corresponding node in the constraint graph.
+  void setGraphNode(constraints::ConstraintGraphNode *newNode) { 
+    GraphNode = newNode; 
+  }
+
+  /// Retrieve the index into the constraint graph's list of type variables.
+  unsigned getGraphIndex() const { 
+    assert(GraphNode && "Graph node isn't set");
+    return GraphIndex; 
+  }
+
+  /// Set the index into the constraint graph's list of type variables.
+  void setGraphIndex(unsigned newIndex) { GraphIndex = newIndex; }
+  
   /// \brief Check whether this type variable either has a representative that
   /// is not itself or has a fixed type binding.
   bool hasRepresentativeOrFixed() const {

lib/Sema/TypeCheckConstraints.cpp

@@ -91,7 +91,6 @@ ConstraintSystem::ConstraintSystem(TypeChecker &tc, DeclContext *dc)
   assert(DC && "context required");
 
   // Create the constraint graph.
-  // FIXME: Enable this by default.
   CG = new ConstraintGraph(*this);
 }
 

lib/Sema/TypeCheckConstraintsSolver.cpp

@@ -890,61 +890,6 @@ static bool tryTypeVariableBindings(
   return !anySolved;
 }
 
-/// Verify that the connected components we computed are identical to
-/// what we would have computed had the constraint graph been built
-/// from scratch right now.
-static void LLVM_ATTRIBUTE_USED verifyConnectedComponents(ConstraintGraph &cg1,
-                                      ArrayRef<TypeVariableType *> typeVars1, 
-                                      ArrayRef<unsigned> components1) {
-  // Build a new constraint graph from scratch.
-  auto &cs = cg1.getConstraintSystem();
-  ConstraintGraph cg2(cs);
-  for (auto typeVar : cs.getTypeVariables())
-    (void)cg2[typeVar];
-  for (auto &constraint : cs.getConstraints())
-    cg2.addConstraint(&constraint);
-
-  // Compute connected components for the newly-created constraint
-  // graph.
-  SmallVector<TypeVariableType *, 16> typeVars2;
-  SmallVector<unsigned, 16> components2;
-  cg2.computeConnectedComponents(typeVars2, components2);
-  
-  // Create a mapping from type variables to components.
-  llvm::SmallDenseMap<TypeVariableType *, unsigned, 4> typeVarToComponentMap1;
-  for (unsigned i = 0, n = typeVars1.size(); i != n; ++i)
-    typeVarToComponentMap1[typeVars1[i]] = components1[i];
-
-  // Create a mapping from components to components
-  llvm::SmallDenseMap<unsigned, unsigned> componentsMap;
-  for (unsigned i = 0, n = typeVars2.size(); i != n; ++i) {
-    // Find the from/to components for this type variable.
-    auto typeVar = typeVars2[i];
-    assert(typeVarToComponentMap1.count(typeVar) && "Missing type variable?");
-    unsigned fromComponent = typeVarToComponentMap1[typeVar];
-    unsigned toComponent = components2[i];
-    
-    // If we haven't mapped the 'from' component yet, add the mapping.
-    auto known = componentsMap.find(fromComponent);
-    if (known == componentsMap.end()) {
-      componentsMap[fromComponent] = toComponent;
-      continue;
-    }
-
-    // Check the mapping.
-    if (known->second != toComponent) {
-      llvm::errs() << "Inconsistent connected components in constraint graphs\n";
-      llvm::errs() << "Evolved constraint graph:\n";
-      cg1.print(llvm::errs());
-      cg1.printConnectedComponents(llvm::errs());
-      llvm::errs() << "Newly-constructed constraint graph:\n";
-      cg2.print(llvm::errs());
-      cg2.printConnectedComponents(llvm::errs());
-      abort();
-    }
-  }
-}
-
 bool ConstraintSystem::solve(SmallVectorImpl<Solution> &solutions,
                              FreeTypeVariableBinding allowFreeTypeVariables) {
   // If there is no solver state, this is the top-level call. Create solver
@@ -1026,7 +971,6 @@ bool ConstraintSystem::solve(SmallVectorImpl<Solution> &solutions,
 
     // Verify that the constraint graph is valid.
     cg.verify();
-    verifyConnectedComponents(cg, typeVars, components);
 
     log << "---Constraint graph---\n";
     cg.print(log);