Skip to content

[NameLookup] Add UnqualifiedLookupRequest #28294

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
CodaFi merged 6 commits into from
Nov 19, 2019
Merged

[NameLookup] Add UnqualifiedLookupRequest #28294

CodaFi merged 6 commits into from
Nov 19, 2019

Conversation

@hamishknight
Copy link
Contributor

@hamishknight hamishknight commented Nov 15, 2019

This PR adds a request to perform unqualified lookup, and uses it to replace the UnqualifiedLookup type.

@hamishknight
NFC: Move LookupResult from Sema to Name Lookup
185c265 
This will be used as the result type for the
unqualified lookup request.
@hamishknight
Add UnqualifiedLookupRequest
633de02 
This request performs raw unqualified lookup,
and will be used to replace the UnqualifiedLookup
type.
@hamishknight
Switch UnqualifiedLookup callers over to UnqualifiedLookupRequest
3aa7158
@hamishknight
Remove UnqualifiedLookup
e8c30ca 
Now that we have UnqualifiedLookupRequest, this
class no longer serves much of a purpose. Inline
its constructor logic into the request.
@hamishknight
Remove NumUnqualifiedLookup counter
acbf0b2 
We now have an equivalent counter for
UnqualifiedLookupRequest.
@hamishknight hamishknight requested a review from CodaFi November 15, 2019 22:43
@hamishknight
Copy link
Contributor Author

hamishknight commented Nov 15, 2019

@swift-ci please test

CodaFi
CodaFi reviewed Nov 15, 2019
View reviewed changes
CodaFi
CodaFi reviewed Nov 15, 2019
View reviewed changes
CodaFi
CodaFi approved these changes Nov 15, 2019
View reviewed changes
Copy link
Contributor

@CodaFi CodaFi left a comment

Choose a reason for hiding this comment

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

Nothing but nits. This is a fantastic foundation to build on.

@CodaFi CodaFi requested a review from DougGregor November 15, 2019 23:25
@hamishknight
Add type to package inputs to UnqualifiedLookupRequest
2853615 
This allows us use an OptionSet parameter for
the request (as currently we can't directly use it
as a parameter due to not having an == definition
for it). It also allows us to regain default
arguments for the source loc and flag parameters.
@CodaFi
Copy link
Contributor

CodaFi commented Nov 18, 2019

@swift-ci please test

@hamishknight
Copy link
Contributor Author

hamishknight commented Nov 19, 2019

@swift-ci please smoke test compiler performance

@CodaFi
Copy link
Contributor

CodaFi commented Nov 19, 2019

Let's take these changes and revisit the perf numbers tomorrow morning.

⛵️

@CodaFi CodaFi merged commit a5d8f7f into swiftlang:master Nov 19, 2019
@hamishknight hamishknight deleted the disqualified branch November 19, 2019 02:17
@swift-ci
Copy link
Contributor

swift-ci commented Nov 19, 2019

Summary for master smoketest

Unexpected test results, excluded stats for ReactiveCocoa

Regressions found (see below)

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 121,257,947,445 121,566,915,142 308,967,697 0.25%
LLVM.NumLLVMBytesOutput 6,149,872 6,149,960 88 0.0%
time.swift-driver.wall 15.1s 15.0s -21.9ms -0.15%

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 1,038 1,038 0 0.0%
AST.NumTotalClangImportedEntities 3,904 3,904 0 0.0%
IRModule.NumIRBasicBlocks 17,929 17,929 0 0.0%
IRModule.NumIRFunctions 10,615 10,615 0 0.0%
IRModule.NumIRGlobals 8,800 8,800 0 0.0%
IRModule.NumIRInsts 309,385 309,385 0 0.0%
IRModule.NumIRValueSymbols 18,439 18,439 0 0.0%
LLVM.NumLLVMBytesOutput 6,149,872 6,149,960 88 0.0%
SILModule.NumSILGenFunctions 5,372 5,372 0 0.0%
SILModule.NumSILOptFunctions 7,270 7,270 0 0.0%
Sema.NumConformancesDeserialized 13,112 13,112 0 0.0%
Sema.NumConstraintScopes 38,960 38,960 0 0.0%
Sema.NumDeclsDeserialized 136,769 136,769 0 0.0%
Sema.NumFunctionsTypechecked 2,034 2,034 0 0.0%
Sema.NumGenericSignatureBuilders 4,552 4,552 0 0.0%
Sema.NumLazyIterableDeclContexts 23,533 23,533 0 0.0%
Sema.NumTypesDeserialized 51,870 51,870 0 0.0%
Sema.NumTypesValidated 6,054 6,054 0 0.0%

Release

release brief

Regressed (2)
name old new delta delta_pct
Frontend.NumInstructionsExecuted 148,002,724,604 149,510,766,666 1,508,042,062 1.02% ⛔
time.swift-driver.wall 28.1s 28.4s 302.1ms 1.08% ⛔
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 6,903,212 6,903,268 56 0.0%

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 18,189 18,189 0 0.0%
IRModule.NumIRFunctions 10,044 10,044 0 0.0%
IRModule.NumIRGlobals 8,917 8,917 0 0.0%
IRModule.NumIRInsts 200,801 200,801 0 0.0%
IRModule.NumIRValueSymbols 18,137 18,137 0 0.0%
LLVM.NumLLVMBytesOutput 6,903,212 6,903,268 56 0.0%
SILModule.NumSILGenFunctions 4,178 4,178 0 0.0%
SILModule.NumSILOptFunctions 6,342 6,342 0 0.0%
Sema.NumConformancesDeserialized 11,521 11,521 0 0.0%
Sema.NumConstraintScopes 38,748 38,748 0 0.0%
Sema.NumDeclsDeserialized 32,007 32,007 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,418 16,418 0 0.0%
Sema.NumTypesValidated 3,914 3,914 0 0.0%

@CodaFi
Copy link
Contributor

CodaFi commented Nov 19, 2019

Yup, noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CodaFi CodaFi CodaFi approved these changes

@DougGregor DougGregor Awaiting requested review from DougGregor

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 @CodaFi @swift-ci