[Perf Experiment][ConstraintGraph] Gather constraints improvements #18281

xedin · 2018-07-27T07:11:19Z

Use set to gather constraints for type variables to avoid duplicates
Add filtering to gatherConstraints per type variable

Since it's possible to find the same constraint through two different but equivalent type variables, let's use a set to store constraints instead of a vector to avoid processing the same constraint multiple times.

Most of the use-cases of `gatherConstraints` require filtering at least based on the constraint kind that caller is interested in, so instead of returning unrelated results and asking caller to filter separately, let's add that functionality directly to `gatherConstraints`.

xedin · 2018-07-27T07:11:37Z

@swift-ci please smoke test compiler performance

rudkx · 2018-07-27T07:33:52Z

lib/Sema/CSBindings.cpp

@@ -361,8 +361,7 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) {
  assert(!typeVar->getImpl().getFixedType(nullptr) && "has a fixed type");

  // Gather the constraints associated with this type variable.
-  SmallVector<Constraint *, 8> constraints;
-  llvm::SmallPtrSet<Constraint *, 4> visitedConstraints;
+  SmallPtrSet<Constraint *, 8> constraints;


Sets are unordered collections and as a result you have no stable order when iterating over them. This is especially bad with sets of pointers since you're almost certainly guaranteed change in order from run to run.

I can switch it to SmallSetVector which should have insertion order iteration. Will try that next.

Oh it looks like C++'s std::set is guaranteed to use std::less and you're guaranteed an iteration order, but again with pointers as the elements you will not be guaranteed to iterate over the pointed-to elements in the same order from run to run.

swift-ci · 2018-07-27T08:37:18Z

Build comment file:

Summary for master smoketest

Unexpected test results, excluded stats for Kingfisher, ReactiveCocoa

Regressions found (see below)

Debug
- debug brief
- debug detailed
Release
- release brief
- release detailed

Debug

debug brief

Regressed (1)

name	old	new	delta	delta_pct
time.swift-driver.wall	19.4s	19.8s	421.1ms	2.17% ⛔

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (1)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	9,397,020	9,397,020	0	0.0%

debug detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (23)

name	old	new	delta_pct
AST.NumImportedExternalDefinitions	5,978	5,978	0.0%
AST.NumLoadedModules	1,281	1,281	0.0%
AST.NumTotalClangImportedEntities	17,774	17,774	0.0%
AST.NumUsedConformances	1,285	1,285	0.0%
IRModule.NumIRBasicBlocks	31,418	31,418	0.0%
IRModule.NumIRFunctions	17,223	17,223	0.0%
IRModule.NumIRGlobals	13,993	13,993	0.0%
IRModule.NumIRInsts	439,046	439,046	0.0%
IRModule.NumIRValueSymbols	29,094	29,094	0.0%
LLVM.NumLLVMBytesOutput	9,397,020	9,397,020	0.0%
SILModule.NumSILGenFunctions	8,060	8,060	0.0%
SILModule.NumSILOptFunctions	11,016	11,016	0.0%
Sema.NumConformancesDeserialized	40,982	40,982	0.0%
Sema.NumConstraintScopes	73,633	73,633	0.0%
Sema.NumDeclsDeserialized	258,914	258,914	0.0%
Sema.NumDeclsValidated	27,647	27,647	0.0%
Sema.NumFunctionsTypechecked	5,036	5,036	0.0%
Sema.NumGenericSignatureBuilders	11,918	11,918	0.0%
Sema.NumLazyGenericEnvironments	50,750	50,750	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	5,999	5,999	0.0%
Sema.NumLazyIterableDeclContexts	38,431	38,431	0.0%
Sema.NumTypesDeserialized	276,676	276,676	0.0%
Sema.NumTypesValidated	35,914	35,914	0.0%

Release

release brief

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (2)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	10,553,536	10,553,536	0	0.0%
time.swift-driver.wall	32.5s	32.7s	213.1ms	0.66%

release detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (23)

name	old	new	delta_pct
AST.NumImportedExternalDefinitions	1,411	1,411	0.0%
AST.NumLoadedModules	100	100	0.0%
AST.NumTotalClangImportedEntities	4,902	4,902	0.0%
AST.NumUsedConformances	1,287	1,287	0.0%
IRModule.NumIRBasicBlocks	34,271	34,271	0.0%
IRModule.NumIRFunctions	15,369	15,369	0.0%
IRModule.NumIRGlobals	13,630	13,630	0.0%
IRModule.NumIRInsts	338,917	338,917	0.0%
IRModule.NumIRValueSymbols	27,267	27,267	0.0%
LLVM.NumLLVMBytesOutput	10,553,536	10,553,536	0.0%
SILModule.NumSILGenFunctions	6,203	6,203	0.0%
SILModule.NumSILOptFunctions	9,396	9,396	0.0%
Sema.NumConformancesDeserialized	19,784	19,784	0.0%
Sema.NumConstraintScopes	72,201	72,201	0.0%
Sema.NumDeclsDeserialized	55,058	55,058	0.0%
Sema.NumDeclsValidated	20,786	20,786	0.0%
Sema.NumFunctionsTypechecked	3,079	3,079	0.0%
Sema.NumGenericSignatureBuilders	2,835	2,835	0.0%
Sema.NumLazyGenericEnvironments	9,815	9,815	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	1,382	1,382	0.0%
Sema.NumLazyIterableDeclContexts	5,638	5,638	0.0%
Sema.NumTypesDeserialized	71,222	71,222	0.0%
Sema.NumTypesValidated	17,086	17,086	0.0%

For stable iteration order, let's switch from `SmallPtrSet` to `SetVector` which ensures insertion order iteration.

xedin · 2018-07-27T22:44:53Z

@swift-ci please smoke test compiler performance

DougGregor

Definitely cleaner!

DougGregor · 2018-07-27T22:52:10Z

lib/Sema/CSDiag.cpp

+          tv, constraints, ConstraintGraph::GatheringKind::EquivalenceClass,
+          [&](Constraint *constraint) -> bool {
+            // We are not interested in ConformsTo constraints because
+            // such constraints specify restrictions on the archetypes


I'd rather not say "archetypes" here; I think the reason we don't care about conformsTo constraints is that we can't derive any concrete type information from them.

Sounds good! It's an existing comment I've moved here but I'll fix it up if perf test is green...

swift-ci · 2018-07-28T00:06:14Z

Build comment file:

Summary for master smoketest

Unexpected test results, excluded stats for Kingfisher, ReactiveCocoa

No regressions above thresholds

Debug
- debug brief
- debug detailed
Release
- release brief
- release detailed

Debug

debug brief

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (2)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	9,396,312	9,396,312	0	0.0%
time.swift-driver.wall	22.1s	22.1s	-63.9ms	-0.29%

debug detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (23)

name	old	new	delta_pct
AST.NumImportedExternalDefinitions	6,105	6,105	0.0%
AST.NumLoadedModules	1,510	1,510	0.0%
AST.NumTotalClangImportedEntities	17,980	17,980	0.0%
AST.NumUsedConformances	1,285	1,285	0.0%
IRModule.NumIRBasicBlocks	31,418	31,418	0.0%
IRModule.NumIRFunctions	17,223	17,223	0.0%
IRModule.NumIRGlobals	13,993	13,993	0.0%
IRModule.NumIRInsts	439,046	439,046	0.0%
IRModule.NumIRValueSymbols	29,094	29,094	0.0%
LLVM.NumLLVMBytesOutput	9,396,312	9,396,312	0.0%
SILModule.NumSILGenFunctions	8,060	8,060	0.0%
SILModule.NumSILOptFunctions	11,016	11,016	0.0%
Sema.NumConformancesDeserialized	43,451	43,451	0.0%
Sema.NumConstraintScopes	73,637	73,637	0.0%
Sema.NumDeclsDeserialized	274,701	274,701	0.0%
Sema.NumDeclsValidated	28,268	28,268	0.0%
Sema.NumFunctionsTypechecked	5,036	5,036	0.0%
Sema.NumGenericSignatureBuilders	12,579	12,579	0.0%
Sema.NumLazyGenericEnvironments	54,673	54,673	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	6,373	6,373	0.0%
Sema.NumLazyIterableDeclContexts	42,214	42,214	0.0%
Sema.NumTypesDeserialized	294,072	294,072	0.0%
Sema.NumTypesValidated	37,785	37,785	0.0%

Release

release brief

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (2)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	10,553,536	10,553,536	0	0.0%
time.swift-driver.wall	36.4s	36.6s	187.2ms	0.51%

release detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (0)

name	old	new	delta	delta_pct

Unchanged (delta < 1.0% or delta < 100.0ms) (23)

name	old	new	delta_pct
AST.NumImportedExternalDefinitions	1,411	1,411	0.0%
AST.NumLoadedModules	100	100	0.0%
AST.NumTotalClangImportedEntities	4,902	4,902	0.0%
AST.NumUsedConformances	1,287	1,287	0.0%
IRModule.NumIRBasicBlocks	34,271	34,271	0.0%
IRModule.NumIRFunctions	15,369	15,369	0.0%
IRModule.NumIRGlobals	13,630	13,630	0.0%
IRModule.NumIRInsts	338,917	338,917	0.0%
IRModule.NumIRValueSymbols	27,267	27,267	0.0%
LLVM.NumLLVMBytesOutput	10,553,536	10,553,536	0.0%
SILModule.NumSILGenFunctions	6,203	6,203	0.0%
SILModule.NumSILOptFunctions	9,396	9,396	0.0%
Sema.NumConformancesDeserialized	19,784	19,784	0.0%
Sema.NumConstraintScopes	72,201	72,201	0.0%
Sema.NumDeclsDeserialized	55,070	55,070	0.0%
Sema.NumDeclsValidated	20,786	20,786	0.0%
Sema.NumFunctionsTypechecked	3,079	3,079	0.0%
Sema.NumGenericSignatureBuilders	2,835	2,835	0.0%
Sema.NumLazyGenericEnvironments	9,815	9,815	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	1,382	1,382	0.0%
Sema.NumLazyIterableDeclContexts	5,638	5,638	0.0%
Sema.NumTypesDeserialized	71,244	71,244	0.0%
Sema.NumTypesValidated	17,086	17,086	0.0%

…rchetype errors

xedin · 2018-07-28T02:57:41Z

@swift-ci please smoke test

xedin · 2018-07-28T02:57:55Z

@swift-ci please test source compatibility

xedin · 2018-07-28T07:27:33Z

Failure in SwiftNIO is unrelated to these changes.

xedin added 2 commits July 26, 2018 22:41

[ConstraintGraph] Use set to gather constraints for type variables

dd798ac

Since it's possible to find the same constraint through two different but equivalent type variables, let's use a set to store constraints instead of a vector to avoid processing the same constraint multiple times.

rudkx reviewed Jul 27, 2018

View reviewed changes

[ConstraintGraph] Change gatherConstraints to take SetVector

624c183

For stable iteration order, let's switch from `SmallPtrSet` to `SetVector` which ensures insertion order iteration.

xedin requested a review from DougGregor July 27, 2018 22:45

DougGregor approved these changes Jul 27, 2018

View reviewed changes

[Diagnostics] NFC: Clarify why we don't care about while diagnosing a…

d9f0134

…rchetype errors

xedin merged commit ae4e106 into swiftlang:master Jul 28, 2018

[Perf Experiment][ConstraintGraph] Gather constraints improvements #18281

[Perf Experiment][ConstraintGraph] Gather constraints improvements #18281

Uh oh!

Conversation

xedin commented Jul 27, 2018

Uh oh!

xedin commented Jul 27, 2018

Uh oh!

rudkx Jul 27, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Jul 27, 2018

Choose a reason for hiding this comment

Uh oh!

rudkx Jul 27, 2018

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Jul 27, 2018

Build comment file:

Summary for master smoketest

Debug

debug brief

debug detailed

Release

release brief

release detailed

Uh oh!

xedin commented Jul 27, 2018

Uh oh!

DougGregor left a comment

Choose a reason for hiding this comment

Uh oh!

DougGregor Jul 27, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Jul 27, 2018

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Jul 28, 2018

Build comment file:

Summary for master smoketest

Debug

debug brief

debug detailed

Release

release brief

release detailed

Uh oh!

xedin commented Jul 28, 2018

Uh oh!

xedin commented Jul 28, 2018

Uh oh!

xedin commented Jul 28, 2018

Uh oh!

Uh oh!