Skip to content

Shrink AST DenseMaps #23323

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.

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

Merged
merged 2 commits into from
Mar 15, 2019
Merged

Conversation

davezarzycki
Copy link
Contributor

Memory saves are generally good. Also, searching a DenseMap for a llvm::PointerIntPair is slightly faster than a std:pair.

@davezarzycki
Copy link
Contributor Author

@swift-ci please smoke test

@davezarzycki davezarzycki requested a review from rudkx March 15, 2019 15:05
@CodaFi
Copy link
Contributor

CodaFi commented Mar 15, 2019

With luck we’ll live in a world with only “simple” paren types.

LGTM

@jrose-apple
Copy link
Contributor

@swift-ci Please smoke test compiler performance

@jrose-apple
Copy link
Contributor

You can merge ahead of my perf tests. I just want to see if anything shows up.

(By the way, Mark is no longer working on Swift these days. :-( )

@davezarzycki davezarzycki merged commit 7f0d756 into swiftlang:master Mar 15, 2019
@davezarzycki davezarzycki deleted the shrink_ast_densemaps branch March 15, 2019 16:48
@davezarzycki
Copy link
Contributor Author

@jrose-apple – Outside of contrived or pathological scenarios, I doubt this will impact the perf suite. It's just the right thing to do. On the topic of code review, I really wish we'd reconsider the "CODE_OWNERS.txt" proposal. It doesn't need GitHub enforcement, just up to date so that GitHub can recommend code reviewers.

@davezarzycki
Copy link
Contributor Author

More so, as an outsider, the GitHub's attempt at guessing the "right" reviewers isn't helpful most of the time. It seems like a crude "who touched these lines last" algorithm.

@jrose-apple
Copy link
Contributor

I think that's exactly what it is. Still better than nothing, but yeah, not as good as something more specified.

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.

LGTM as well!

@swift-ci
Copy link
Contributor

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 12.4s 13.0s 668.7ms 5.4% ⛔
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 98,974,132,660 98,978,021,034 3,888,374 0.0%
LLVM.NumLLVMBytesOutput 6,142,216 6,142,096 -120 -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 delta_pct
AST.NumImportedExternalDefinitions 1,032 1,032 0 0.0%
AST.NumLoadedModules 828 828 0 0.0%
AST.NumTotalClangImportedEntities 4,210 4,210 0 0.0%
AST.NumUsedConformances 886 886 0 0.0%
IRModule.NumIRBasicBlocks 18,032 18,032 0 0.0%
IRModule.NumIRFunctions 10,554 10,554 0 0.0%
IRModule.NumIRGlobals 8,013 8,013 0 0.0%
IRModule.NumIRInsts 312,435 312,435 0 0.0%
IRModule.NumIRValueSymbols 17,630 17,630 0 0.0%
LLVM.NumLLVMBytesOutput 6,142,216 6,142,096 -120 -0.0%
SILModule.NumSILGenFunctions 5,382 5,382 0 0.0%
SILModule.NumSILOptFunctions 7,055 7,055 0 0.0%
Sema.NumConformancesDeserialized 16,171 16,171 0 0.0%
Sema.NumConstraintScopes 40,202 40,202 0 0.0%
Sema.NumDeclsDeserialized 134,611 134,611 0 0.0%
Sema.NumDeclsValidated 11,208 11,208 0 0.0%
Sema.NumFunctionsTypechecked 2,516 2,516 0 0.0%
Sema.NumGenericSignatureBuilders 4,776 4,776 0 0.0%
Sema.NumLazyGenericEnvironments 30,011 30,011 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 2,012 2,012 0 0.0%
Sema.NumLazyIterableDeclContexts 23,862 23,862 0 0.0%
Sema.NumTypesDeserialized 59,970 59,970 0 0.0%
Sema.NumTypesValidated 11,902 11,902 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 140,679,406,443 140,752,987,359 73,580,916 0.05%
LLVM.NumLLVMBytesOutput 7,131,896 7,131,952 56 0.0%
time.swift-driver.wall 23.4s 23.4s -4.0ms -0.02%

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 402 402 0 0.0%
AST.NumLoadedModules 76 76 0 0.0%
AST.NumTotalClangImportedEntities 2,146 2,146 0 0.0%
AST.NumUsedConformances 890 890 0 0.0%
IRModule.NumIRBasicBlocks 20,921 20,921 0 0.0%
IRModule.NumIRFunctions 10,070 10,070 0 0.0%
IRModule.NumIRGlobals 8,102 8,102 0 0.0%
IRModule.NumIRInsts 220,156 220,156 0 0.0%
IRModule.NumIRValueSymbols 17,316 17,316 0 0.0%
LLVM.NumLLVMBytesOutput 7,131,896 7,131,952 56 0.0%
SILModule.NumSILGenFunctions 4,178 4,178 0 0.0%
SILModule.NumSILOptFunctions 5,857 5,857 0 0.0%
Sema.NumConformancesDeserialized 12,445 12,445 0 0.0%
Sema.NumConstraintScopes 39,280 39,280 0 0.0%
Sema.NumDeclsDeserialized 32,447 32,447 0 0.0%
Sema.NumDeclsValidated 7,874 7,874 0 0.0%
Sema.NumFunctionsTypechecked 2,204 2,204 0 0.0%
Sema.NumGenericSignatureBuilders 1,718 1,718 0 0.0%
Sema.NumLazyGenericEnvironments 6,873 6,873 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 130 130 0 0.0%
Sema.NumLazyIterableDeclContexts 4,259 4,259 0 0.0%
Sema.NumTypesDeserialized 18,170 18,170 0 0.0%
Sema.NumTypesValidated 6,942 6,942 0 0.0%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants