Skip to content

[CS] Use a MapVector to cache resolved overloads #28593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Dec 6, 2019
Merged

[CS] Use a MapVector to cache resolved overloads #28593

merged 2 commits into from
Dec 6, 2019

Conversation

hamishknight
Copy link
Contributor

Rather than maintaining a linked list of overload choices, which must be linearly searched each time we need to lookup an overload at a given callee locator, use a MapVector which can be rolled back at the end of a scope.

Remove ResolvedOverloadSetListItem in favor of using SelectedOverload, which avoids the need to convert between them when moving from ConstraintSystem to Solution.

@hamishknight hamishknight requested a review from xedin December 5, 2019 19:36
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight
Copy link
Contributor Author

@swift-ci please test source compatibility

@hamishknight
Copy link
Contributor Author

@swift-ci please smoke test compiler performance

xedin
xedin approved these changes Dec 5, 2019
View reviewed changes
Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I have left one meta comment inline and I'm very excited to see ResolvedOverloadSetListItem go away!

@swift-ci
Copy link
Contributor

swift-ci commented Dec 5, 2019

Summary for master smoketest

Unexpected test results, excluded stats for ReactiveCocoa

Regressions found (see below)

Debug

debug brief

Regressed (1)
name old new delta delta_pct
time.swift-driver.wall 9.4s 9.5s 107.1ms 1.14% ⛔
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (2)
name old new delta delta_pct
Frontend.NumInstructionsExecuted 104,735,774,434 104,764,505,594 28,731,160 0.03%
LLVM.NumLLVMBytesOutput 6,148,160 6,148,168 8 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) (18)
name old new delta delta_pct
AST.NumLoadedModules 660 660 0 0.0%
AST.NumTotalClangImportedEntities 3,666 3,666 0 0.0%
IRModule.NumIRBasicBlocks 17,929 17,929 0 0.0%
IRModule.NumIRFunctions 10,597 10,597 0 0.0%
IRModule.NumIRGlobals 8,820 8,820 0 0.0%
IRModule.NumIRInsts 309,157 309,157 0 0.0%
IRModule.NumIRValueSymbols 18,441 18,441 0 0.0%
LLVM.NumLLVMBytesOutput 6,148,160 6,148,168 8 0.0%
SILModule.NumSILGenFunctions 5,372 5,372 0 0.0%
SILModule.NumSILOptFunctions 7,270 7,270 0 0.0%
Sema.NumConformancesDeserialized 11,782 11,782 0 0.0%
Sema.NumConstraintScopes 38,992 38,992 0 0.0%
Sema.NumDeclsDeserialized 115,226 115,226 0 0.0%
Sema.NumFunctionsTypechecked 2,034 2,034 0 0.0%
Sema.NumGenericSignatureBuilders 4,068 4,068 0 0.0%
Sema.NumLazyIterableDeclContexts 18,531 18,531 0 0.0%
Sema.NumTypesDeserialized 45,676 45,676 0 0.0%
Sema.NumTypesValidated 5,918 5,918 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 147,331,899,618 147,368,048,596 36,148,978 0.02%
LLVM.NumLLVMBytesOutput 6,864,920 6,864,848 -72 -0.0%
time.swift-driver.wall 17.9s 17.9s -40.2ms -0.22%

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) (18)
name old new delta delta_pct
AST.NumLoadedModules 76 76 0 0.0%
AST.NumTotalClangImportedEntities 2,136 2,136 0 0.0%
IRModule.NumIRBasicBlocks 17,963 17,963 0 0.0%
IRModule.NumIRFunctions 9,916 9,916 0 0.0%
IRModule.NumIRGlobals 8,921 8,921 0 0.0%
IRModule.NumIRInsts 200,125 200,125 0 0.0%
IRModule.NumIRValueSymbols 18,015 18,015 0 0.0%
LLVM.NumLLVMBytesOutput 6,864,920 6,864,848 -72 -0.0%
SILModule.NumSILGenFunctions 4,178 4,178 0 0.0%
SILModule.NumSILOptFunctions 6,229 6,229 0 0.0%
Sema.NumConformancesDeserialized 11,521 11,521 0 0.0%
Sema.NumConstraintScopes 38,794 38,794 0 0.0%
Sema.NumDeclsDeserialized 32,115 32,115 0 0.0%
Sema.NumFunctionsTypechecked 2,034 2,034 0 0.0%
Sema.NumGenericSignatureBuilders 1,579 1,579 0 0.0%
Sema.NumLazyIterableDeclContexts 4,267 4,267 0 0.0%
Sema.NumTypesDeserialized 16,745 16,745 0 0.0%
Sema.NumTypesValidated 3,914 3,914 0 0.0%

@hamishknight
[CS] Use a MapVector to cache resolved overloads
a97328d 
Rather than maintaining a linked list of overload
choices, which must be linearly searched each time
we need to lookup an overload at a given callee
locator, use a MapVector which can be rolled back
at the end of a scope.

Remove ResolvedOverloadSetListItem in favor of
using SelectedOverload, which avoids the need to
convert between them when moving from
ConstraintSystem to Solution.
@hamishknight
[CS] Use different locators for nested 'wrappedValue' lookups
40750c0 
With the new way of caching overloads in the
constraint system, we no longer allow different
overloads to be resolved for the same locator in
the same scope.

Adjust the property wrapper logic
so that it appends Member locator elements for
nested lookups.
@hamishknight
Copy link
Contributor Author

@swift-ci please smoke test

@hamishknight hamishknight merged commit c32e31c into swiftlang:master Dec 6, 2019
@hamishknight hamishknight deleted the cache-in-hand branch December 6, 2019 01:25
@hamishknight hamishknight mentioned this pull request Dec 6, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xedin xedin xedin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hamishknight @swift-ci @xedin