[CSBindings] Cache already computed type variable bindings #19852

xedin · 2018-10-12T07:31:03Z

To avoid re-computing same bindings over and over again,
let's try to keep a cache of active bindings and invalidate
them as needed.

Invalidate bindings conservatively when new constraints are
introduced (even if such constraints are not going to result
in new type bindings), and when type variables are assigned
fixed types or merged together.

Invalidation also makes sure to erase bindings of all adjacent
type variables related to equivalence class of the variables
in question.

This significantly speeds up use-cases which have a lot of type
variables such as collection literals with arbitrary nesting.

xedin · 2018-10-12T07:31:20Z

@swift-ci please smoke test compiler performance

slavapestov

Some optional comments, but I don’t claim to understand this part of the type checker too well

slavapestov · 2018-10-12T07:33:50Z

validation-test/Sema/type_checker_perf/slow/rdar30596744_2.swift.gyb

@@ -1,4 +1,4 @@
-// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select incrementScopeCounter %s --expected-exit-code 1
+// RUN: %scale-test --invert-result --begin 1 --end 4 --step 1 --select incrementScopeCounter %s --expected-exit-code 1


Why did you change this?

Because scale test can’t fit the curve with these changes unfortunately...

slavapestov · 2018-10-12T07:34:55Z

lib/Sema/CSBindings.cpp

    if (auto bindings = getPotentialBindings(typeVar))
-      cache.insert({typeVar, std::move(bindings)});
+      Bindings.insert({typeVar, std::move(bindings)});


Do you want to assert that the insertion succeeded?

slavapestov · 2018-10-12T07:35:30Z

lib/Sema/CSSimplify.cpp

@@ -1561,6 +1561,7 @@ ConstraintSystem::matchTypesBindTypeVar(
  }

  assignFixedType(typeVar, type);
+  invalidateBindings(typeVar);


Are there any other callers of assignFixedType() that need to do this?

I am considering unifying both assign and merge paths, want to run perf test first before I spend more time on this...

slavapestov · 2018-10-12T07:35:51Z

lib/Sema/CSSimplify.cpp

@@ -1688,6 +1689,7 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,

        // Merge the equivalence classes corresponding to these two variables.
        mergeEquivalenceClasses(rep1, rep2);
+        invalidateBindings(rep1);


Maybe do this in mergeEquivalenceClasses()?

lib/Sema/CSBindings.cpp

swift-ci · 2018-10-12T08:57:33Z

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) (3)

name	old	new	delta	delta_pct
Frontend.NumInstructionsExecuted	167,425,092,535	167,656,498,085	231,405,550	0.14%
LLVM.NumLLVMBytesOutput	9,520,188	9,520,188	0	0.0%
time.swift-driver.wall	21.1s	21.0s	-103.8ms	-0.49%

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	delta_pct
AST.NumImportedExternalDefinitions	4,461	4,461	0	0.0%
AST.NumLoadedModules	1,338	1,338	0	0.0%
AST.NumTotalClangImportedEntities	17,614	17,614	0	0.0%
AST.NumUsedConformances	1,352	1,352	0	0.0%
IRModule.NumIRBasicBlocks	32,442	32,442	0	0.0%
IRModule.NumIRFunctions	16,460	16,460	0	0.0%
IRModule.NumIRGlobals	14,175	14,175	0	0.0%
IRModule.NumIRInsts	443,509	443,509	0	0.0%
IRModule.NumIRValueSymbols	29,370	29,370	0	0.0%
LLVM.NumLLVMBytesOutput	9,520,188	9,520,188	0	0.0%
SILModule.NumSILGenFunctions	7,843	7,843	0	0.0%
SILModule.NumSILOptFunctions	10,695	10,695	0	0.0%
Sema.NumConformancesDeserialized	22,077	22,077	0	0.0%
Sema.NumConstraintScopes	70,304	70,300	-4	-0.01%
Sema.NumDeclsDeserialized	214,844	214,844	0	0.0%
Sema.NumDeclsValidated	16,273	16,273	0	0.0%
Sema.NumFunctionsTypechecked	6,058	6,058	0	0.0%
Sema.NumGenericSignatureBuilders	7,103	7,103	0	0.0%
Sema.NumLazyGenericEnvironments	48,291	48,291	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	2,333	2,333	0	0.0%
Sema.NumLazyIterableDeclContexts	39,816	39,816	0	0.0%
Sema.NumTypesDeserialized	91,936	91,936	0	0.0%
Sema.NumTypesValidated	13,937	13,937	0	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) (3)

name	old	new	delta	delta_pct
Frontend.NumInstructionsExecuted	222,171,391,941	222,251,820,743	80,428,802	0.04%
LLVM.NumLLVMBytesOutput	10,644,604	10,644,588	-16	-0.0%
time.swift-driver.wall	36.9s	36.9s	-14.6ms	-0.04%

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	delta_pct
AST.NumImportedExternalDefinitions	1,111	1,111	0	0.0%
AST.NumLoadedModules	105	105	0	0.0%
AST.NumTotalClangImportedEntities	4,574	4,574	0	0.0%
AST.NumUsedConformances	1,354	1,354	0	0.0%
IRModule.NumIRBasicBlocks	34,584	34,584	0	0.0%
IRModule.NumIRFunctions	15,008	15,008	0	0.0%
IRModule.NumIRGlobals	13,708	13,708	0	0.0%
IRModule.NumIRInsts	333,412	333,412	0	0.0%
IRModule.NumIRValueSymbols	27,662	27,662	0	0.0%
LLVM.NumLLVMBytesOutput	10,644,604	10,644,588	-16	-0.0%
SILModule.NumSILGenFunctions	6,076	6,076	0	0.0%
SILModule.NumSILOptFunctions	9,041	9,041	0	0.0%
Sema.NumConformancesDeserialized	15,090	15,090	0	0.0%
Sema.NumConstraintScopes	69,419	69,415	-4	-0.01%
Sema.NumDeclsDeserialized	45,610	45,610	0	0.0%
Sema.NumDeclsValidated	10,362	10,362	0	0.0%
Sema.NumFunctionsTypechecked	4,162	4,162	0	0.0%
Sema.NumGenericSignatureBuilders	2,079	2,079	0	0.0%
Sema.NumLazyGenericEnvironments	10,068	10,068	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	201	201	0	0.0%
Sema.NumLazyIterableDeclContexts	5,826	5,826	0	0.0%
Sema.NumTypesDeserialized	26,675	26,675	0	0.0%
Sema.NumTypesValidated	7,103	7,103	0	0.0%

xedin · 2018-10-12T19:16:25Z

@swift-ci please test source compatibility

xedin · 2018-10-12T22:56:44Z

Failure was:

14:56:39 ld: warning: Could not find auto-linked library 'swiftMetal'
14:56:39 Undefined symbols for architecture x86_64:
14:56:39   "__swift_FORCE_LOAD_$_swiftMetal", referenced from:
14:56:39       __swift_FORCE_LOAD_$_swiftMetal_$_Vision in Vision.o
14:56:39      (maybe you meant: __swift_FORCE_LOAD_$_swiftMetal_$_Vision)
14:56:39 ld: symbol(s) not found for architecture x86_64

Let's try again...

xedin · 2018-10-12T22:57:09Z

@swift-ci please clean test source compatibility

xedin · 2018-10-12T23:00:46Z

@swift-ci please test source compatibility

xedin · 2018-10-13T01:23:19Z

Not sure what's up with this linker error, going to wait for a bit before running the suite again...

xedin · 2018-10-13T01:27:26Z

@rudkx For problems with huge collections of literals, these changes cut type-check time significantly e.g. rdar://problem/26877601 (which used to be stack overflow) it reduced it from 6 minutes to 1:54 on my machine...

rudkx

So is the assumption here that having more types as potential bindings than is strictly necessary always safe? (which seems on the surface to be true)

If so it’s not clear that you actually need to invalidate bindings when merging type variables, but could instead merge bindings.

xedin · 2018-10-15T02:35:01Z

@rudkx Yes, that’s an assumption. I am just trying to take it one step at a time, the problem with updating bindings in-place is that we don’t currently record bindings which bind two type variables directly, so that has to be fixed before update in-place is possible...

xedin · 2018-10-16T18:40:25Z

@swift-ci please test

xedin · 2018-10-16T21:10:11Z

@swift-ci please test source compatibility

xedin · 2018-10-16T23:30:34Z

@swift-ci please test

xedin · 2018-10-17T21:26:01Z

@rudkx If you don't have any objections I'm going to merge this soon...

rudkx · 2018-10-17T21:35:18Z

SGTM!

To avoid re-computing same bindings over and over again, let's try to keep a cache of active bindings and invalidate them as needed. Invalidate bindings conservatively when new constraints are introduced (even if such constraints are not going to result in new type bindings), and when type variables are assigned fixed types or merged together. Invalidation also makes sure to erase bindings of all adjacent type variables related to equivalence class of the variables in question. This significantly speeds up use-cases that have a lot of type variables such as collection literals with arbitrary nesting.

xedin · 2018-10-19T23:02:22Z

@swift-ci please test

xedin · 2018-10-19T23:02:34Z

@swift-ci please test source compatibility

xedin · 2018-10-22T21:02:47Z

@swift-ci please smoke test

xedin · 2018-11-06T22:28:12Z

@swift-ci please test

swift-ci · 2018-11-07T01:10:38Z

Build failed
Swift Test OS X Platform
Git Sha - 18da183

xedin · 2018-11-07T01:42:19Z

@swift-ci please test macOS platform

xedin · 2018-11-28T06:40:51Z

@swift-ci please test source compatibility

xedin requested a review from rudkx October 12, 2018 07:31

slavapestov reviewed Oct 12, 2018

View reviewed changes

rudkx reviewed Oct 15, 2018

View reviewed changes

xedin force-pushed the type-binding-cache branch 2 times, most recently from bd72fc6 to cb887c8 Compare October 16, 2018 23:30

This comment has been minimized.

Sign in to view

xedin force-pushed the type-binding-cache branch from cb887c8 to 6378923 Compare October 19, 2018 22:02

xedin force-pushed the type-binding-cache branch from 6378923 to 18da183 Compare October 19, 2018 23:01

This comment has been minimized.

Sign in to view

xedin mentioned this pull request Aug 16, 2019

[Constraint graph] Gathering "all mentions" constraints means all mentions #26691

Closed

xedin closed this Nov 21, 2019

		@@ -1,4 +1,4 @@
		// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select incrementScopeCounter %s --expected-exit-code 1
		// RUN: %scale-test --invert-result --begin 1 --end 4 --step 1 --select incrementScopeCounter %s --expected-exit-code 1

[CSBindings] Cache already computed type variable bindings #19852

[CSBindings] Cache already computed type variable bindings #19852

Uh oh!

Conversation

xedin commented Oct 12, 2018

Uh oh!

xedin commented Oct 12, 2018

Uh oh!

slavapestov left a comment

Choose a reason for hiding this comment

Uh oh!

slavapestov Oct 12, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Oct 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slavapestov Oct 12, 2018

Choose a reason for hiding this comment

Uh oh!

slavapestov Oct 12, 2018

Choose a reason for hiding this comment

Uh oh!

xedin Oct 12, 2018

Choose a reason for hiding this comment

Uh oh!

slavapestov Oct 12, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

swift-ci commented Oct 12, 2018

Build comment file:

Summary for master smoketest

Debug

debug brief

debug detailed

Release

release brief

release detailed

Uh oh!

xedin commented Oct 12, 2018

Uh oh!

xedin commented Oct 12, 2018

Uh oh!

xedin commented Oct 12, 2018

Uh oh!

xedin commented Oct 12, 2018

Uh oh!

xedin commented Oct 13, 2018

Uh oh!

xedin commented Oct 13, 2018

Uh oh!

rudkx left a comment

Choose a reason for hiding this comment

Uh oh!

xedin commented Oct 15, 2018

Uh oh!

xedin commented Oct 16, 2018

Uh oh!

xedin commented Oct 16, 2018

Uh oh!

xedin commented Oct 16, 2018

Uh oh!

This comment has been minimized.

This comment has been minimized.

xedin commented Oct 17, 2018

Uh oh!

rudkx commented Oct 17, 2018

Uh oh!

xedin commented Oct 19, 2018

Uh oh!

xedin commented Oct 19, 2018

Uh oh!

This comment has been minimized.

This comment has been minimized.

xedin commented Oct 22, 2018

Uh oh!

xedin commented Nov 6, 2018

Uh oh!

xedin Oct 12, 2018 •

edited

Loading