From e021691bae53c626f3e83dc89ebe51a1bcaa46be Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 11 May 2018 15:07:19 -0700 Subject: [PATCH 1/2] [ConstraintGraph] Fix `contractEdges` to gather constraints only once Currently we have this non-optimal behavior in `contractEdges` where for every type variable it gathers constraints for its whole equivalence class before checking if any are "contractable", instead constraints could be gathered/filtered once which removes a lot of useless work. --- lib/Sema/ConstraintGraph.cpp | 140 +++++++++++++++++------------------ lib/Sema/ConstraintSystem.h | 16 ++++ 2 files changed, 83 insertions(+), 73 deletions(-) diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index 1a20c9e0f2942..9150c09acc407 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -649,93 +649,87 @@ static bool shouldContractEdge(ConstraintKind kind) { } bool ConstraintGraph::contractEdges() { - llvm::SetVector> contractions; - - auto tyvars = getTypeVariables(); - auto didContractEdges = false; + SmallVector constraints; + CS.findConstraints(constraints, [](const Constraint &constraint) { + return shouldContractEdge(constraint.getKind()); + }); - for (auto tyvar : tyvars) { - SmallVector constraints; - gatherConstraints(tyvar, constraints, - ConstraintGraph::GatheringKind::EquivalenceClass); + bool didContractEdges = false; + for (auto *constraint : constraints) { + auto kind = constraint->getKind(); - for (auto constraint : constraints) { - auto kind = constraint->getKind(); - // Contract binding edges between type variables. - if (shouldContractEdge(kind)) { - auto t1 = constraint->getFirstType()->getDesugaredType(); - auto t2 = constraint->getSecondType()->getDesugaredType(); + // Contract binding edges between type variables. + assert(shouldContractEdge(kind)); - auto tyvar1 = t1->getAs(); - auto tyvar2 = t2->getAs(); + auto t1 = constraint->getFirstType()->getDesugaredType(); + auto t2 = constraint->getSecondType()->getDesugaredType(); - if (!(tyvar1 && tyvar2)) - continue; + auto tyvar1 = t1->getAs(); + auto tyvar2 = t2->getAs(); - auto isParamBindingConstraint = kind == ConstraintKind::BindParam; - - // If the argument is allowed to bind to `inout`, in general, - // it's invalid to contract the edge between argument and parameter, - // but if we can prove that there are no possible bindings - // which result in attempt to bind `inout` type to argument - // type variable, we should go ahead and allow (temporary) - // contraction, because that greatly helps with performance. - // Such action is valid because argument type variable can - // only get its bindings from related overload, which gives - // us enough information to decided on l-valueness. - if (isParamBindingConstraint && tyvar1->getImpl().canBindToInOut()) { - bool isNotContractable = true; - if (auto bindings = CS.getPotentialBindings(tyvar1)) { - for (auto &binding : bindings.Bindings) { - auto type = binding.BindingType; - isNotContractable = type.findIf([&](Type nestedType) -> bool { - if (auto tv = nestedType->getAs()) { - if (!tv->getImpl().mustBeMaterializable()) - return true; - } - - return nestedType->is(); - }); + if (!(tyvar1 && tyvar2)) + continue; - // If there is at least one non-contractable binding, let's - // not risk contracting this edge. - if (isNotContractable) - break; + auto isParamBindingConstraint = kind == ConstraintKind::BindParam; + + // If the argument is allowed to bind to `inout`, in general, + // it's invalid to contract the edge between argument and parameter, + // but if we can prove that there are no possible bindings + // which result in attempt to bind `inout` type to argument + // type variable, we should go ahead and allow (temporary) + // contraction, because that greatly helps with performance. + // Such action is valid because argument type variable can + // only get its bindings from related overload, which gives + // us enough information to decided on l-valueness. + if (isParamBindingConstraint && tyvar1->getImpl().canBindToInOut()) { + bool isNotContractable = true; + if (auto bindings = CS.getPotentialBindings(tyvar1)) { + for (auto &binding : bindings.Bindings) { + auto type = binding.BindingType; + isNotContractable = type.findIf([&](Type nestedType) -> bool { + if (auto tv = nestedType->getAs()) { + if (!tv->getImpl().mustBeMaterializable()) + return true; } - } + return nestedType->is(); + }); + + // If there is at least one non-contractable binding, let's + // not risk contracting this edge. if (isNotContractable) - continue; + break; } + } - auto rep1 = CS.getRepresentative(tyvar1); - auto rep2 = CS.getRepresentative(tyvar2); - - if (((rep1->getImpl().canBindToLValue() == - rep2->getImpl().canBindToLValue()) || - // Allow l-value contractions when binding parameter types. - isParamBindingConstraint)) { - if (CS.TC.getLangOpts().DebugConstraintSolver) { - auto &log = CS.getASTContext().TypeCheckerDebug->getStream(); - if (CS.solverState) - log.indent(CS.solverState->depth * 2); - - log << "Contracting constraint "; - constraint->print(log, &CS.getASTContext().SourceMgr); - log << "\n"; - } - - // Merge the edges and remove the constraint. - removeEdge(constraint); - if (rep1 != rep2) - CS.mergeEquivalenceClasses(rep1, rep2, /*updateWorkList*/ false); - didContractEdges = true; - } + if (isNotContractable) + continue; + } + + auto rep1 = CS.getRepresentative(tyvar1); + auto rep2 = CS.getRepresentative(tyvar2); + + if (((rep1->getImpl().canBindToLValue() == + rep2->getImpl().canBindToLValue()) || + // Allow l-value contractions when binding parameter types. + isParamBindingConstraint)) { + if (CS.TC.getLangOpts().DebugConstraintSolver) { + auto &log = CS.getASTContext().TypeCheckerDebug->getStream(); + if (CS.solverState) + log.indent(CS.solverState->depth * 2); + + log << "Contracting constraint "; + constraint->print(log, &CS.getASTContext().SourceMgr); + log << "\n"; } + + // Merge the edges and remove the constraint. + removeEdge(constraint); + if (rep1 != rep2) + CS.mergeEquivalenceClasses(rep1, rep2, /*updateWorkList*/ false); + didContractEdges = true; } } - return didContractEdges; } diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 45146957b29f5..a50c18bd04263 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -1909,6 +1909,12 @@ class ConstraintSystem { /// due to a change. ConstraintList &getActiveConstraints() { return ActiveConstraints; } + void findConstraints(SmallVectorImpl &found, + llvm::function_ref pred) { + filterConstraints(ActiveConstraints, pred, found); + filterConstraints(InactiveConstraints, pred, found); + } + /// \brief Retrieve the representative of the equivalence class containing /// this type variable. TypeVariableType *getRepresentative(TypeVariableType *typeVar) { @@ -2074,6 +2080,16 @@ class ConstraintSystem { /// into the worklist. void addTypeVariableConstraintsToWorkList(TypeVariableType *typeVar); + static void + filterConstraints(ConstraintList &constraints, + llvm::function_ref pred, + SmallVectorImpl &found) { + for (auto &constraint : constraints) { + if (pred(constraint)) + found.push_back(&constraint); + } + } + public: /// \brief Coerce the given expression to an rvalue, if it isn't already. From 3e254678a2189df5a69ff9a7df7ecd8e835d5540 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 11 May 2018 22:50:52 -0700 Subject: [PATCH 2/2] [Sema] Add counter to track number of constraints considered by each edge contraction attempt --- include/swift/Basic/Statistics.def | 5 +++++ lib/Sema/ConstraintGraph.cpp | 15 ++++++++++++++- lib/Sema/ConstraintGraph.h | 4 ++++ .../type_checker_perf/fast/rdar29358447.swift.gyb | 10 ++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/rdar29358447.swift.gyb diff --git a/include/swift/Basic/Statistics.def b/include/swift/Basic/Statistics.def index b1e781be765f4..0a64bbedd56b7 100644 --- a/include/swift/Basic/Statistics.def +++ b/include/swift/Basic/Statistics.def @@ -149,6 +149,11 @@ FRONTEND_STATISTIC(Sema, NumConformancesDeserialized) /// expression typechecker did". FRONTEND_STATISTIC(Sema, NumConstraintScopes) +/// Number of constraints considered per attempt to +/// contract constraint graph edges. +/// This is a measure of complexity of the contraction algorithm. +FRONTEND_STATISTIC(Sema, NumConstraintsConsideredForEdgeContraction) + /// Number of declarations that were deserialized. A rough proxy for the amount /// of material loaded from other modules. FRONTEND_STATISTIC(Sema, NumDeclsDeserialized) diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index 9150c09acc407..a493141b3f2dd 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -18,6 +18,7 @@ #include "ConstraintGraph.h" #include "ConstraintGraphScope.h" #include "ConstraintSystem.h" +#include "swift/Basic/Statistic.h" #include "llvm/Support/Debug.h" #include "llvm/Support/SaveAndRestore.h" #include @@ -27,6 +28,8 @@ using namespace swift; using namespace constraints; +#define DEBUG_TYPE "ConstraintGraph" + #pragma mark Graph construction/destruction ConstraintGraph::ConstraintGraph(ConstraintSystem &cs) : CS(cs) { } @@ -650,7 +653,9 @@ static bool shouldContractEdge(ConstraintKind kind) { bool ConstraintGraph::contractEdges() { SmallVector constraints; - CS.findConstraints(constraints, [](const Constraint &constraint) { + CS.findConstraints(constraints, [&](const Constraint &constraint) { + // Track how many constraints did contraction algorithm iterated over. + incrementConstraintsPerContractionCounter(); return shouldContractEdge(constraint.getKind()); }); @@ -767,6 +772,14 @@ void ConstraintGraph::optimize() { while (contractEdges()) {} } +void ConstraintGraph::incrementConstraintsPerContractionCounter() { + SWIFT_FUNC_STAT; + auto &context = CS.getASTContext(); + if (context.Stats) + context.Stats->getFrontendCounters() + .NumConstraintsConsideredForEdgeContraction++; +} + #pragma mark Debugging output void ConstraintGraphNode::print(llvm::raw_ostream &out, unsigned indent) { diff --git a/lib/Sema/ConstraintGraph.h b/lib/Sema/ConstraintGraph.h index d3ccf20a70cb5..328dd76a8eb84 100644 --- a/lib/Sema/ConstraintGraph.h +++ b/lib/Sema/ConstraintGraph.h @@ -332,6 +332,10 @@ class ConstraintGraph { /// Constraints that are "orphaned" because they contain no type variables. SmallVector OrphanedConstraints; + /// Increment the number of constraints considered per attempt + /// to contract constrant graph edges. + void incrementConstraintsPerContractionCounter(); + /// The kind of change made to the graph. enum class ChangeKind { /// Added a type variable. diff --git a/validation-test/Sema/type_checker_perf/fast/rdar29358447.swift.gyb b/validation-test/Sema/type_checker_perf/fast/rdar29358447.swift.gyb new file mode 100644 index 0000000000000..eb134e4bf8f27 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/rdar29358447.swift.gyb @@ -0,0 +1,10 @@ +// RUN: %scale-test --begin 0 --end 25000 --step 1000 --typecheck --select incrementConstraintsPerContractionCounter %s +// REQUIRES: OS=macosx +// REQUIRES: asserts + +let _: [Int] = [ +%for i in range(0, N): + 1, +%end + 1 +]