Skip to content

Remove the "shrink" part of the solver. #17922

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
Closed
wants to merge 2 commits into from
Closed

Remove the "shrink" part of the solver. #17922

wants to merge 2 commits into from

Conversation

rudkx
Copy link
Contributor

@rudkx rudkx commented Jul 13, 2018

At one point this phase was providing a lot of value in reducing the
number of expressions we ultimately decided were too complex, but at
this point it doesn't seem to be helping very often.

Based on experimenting with some of our slow test cases, in several
cases it actually increases the amount of work we do before we decide
an expression is too complex, sometimes making it take 2-3x longer.

I will open a bug to track restoring the two expressions that we are
now "too complex" on, so that the tests can be restored once we fix
some of the underlying problems.

@rudkx
Remove the "shrink" part of the solver.
a77cf23 
At one point this phase was providing a lot of value in reducing the
number of expressions we ultimately decided were too complex, but at
this point it doesn't seem to be helping very often.

Based on experimenting with some of our slow test cases, in several
cases it actually increases the amount of work we do before we decide
an expression is too complex, sometimes making it take 2-3x longer.

I will open a bug to track restoring the two expressions that we are
now "too complex" on, so that the tests can be restored once we fix
some of the underlying problems.
@rudkx
Copy link
Contributor Author

rudkx commented Jul 13, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Jul 13, 2018

@swift-ci Please test source compatibility

@rudkx
Copy link
Contributor Author

rudkx commented Jul 13, 2018

@swift-ci Please test compiler performance

@swift-ci
Copy link
Contributor

Build comment file:

Summary for master full

Unexpected test results, excluded stats for CoreStore, SRP, Kitura, ObjectMapper

Regressions found (see below)

Debug-batch

debug-batch 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 721,792,372 721,812,492 20,120 0.0%
time.swift-driver.wall 1373.8s 1381.1s 7.2s 0.53%

debug-batch detailed

Regressed (2)
name old new delta delta_pct
Sema.NumConstraintScopes 6,577,511 7,210,782 633,271 9.63% ⛔
Sema.NumConstraintsConsideredForEdgeContraction 17,281,963 19,295,788 2,013,825 11.65% ⛔
Improved (3)
name old new delta delta_pct
Sema.NamedLazyMemberLoadSuccessCount 2,409,239 2,376,107 -33,132 -1.38% ✅
Sema.NominalTypeLookupDirectCount 18,088,729 17,865,237 -223,492 -1.24% ✅
Sema.NumDeclsValidated 1,777,903 1,752,521 -25,382 -1.43% ✅
Unchanged (delta < 1.0% or delta < 100.0ms) (75)
name old new delta delta_pct
AST.NumASTBytesAllocated 17,418,485,239 17,455,997,566 37,512,327 0.22%
AST.NumDecls 49,115 49,115 0 0.0%
AST.NumDependencies 108,003 108,003 0 0.0%
AST.NumImportedExternalDefinitions 922,817 923,102 285 0.03%
AST.NumInfixOperators 16,785 16,785 0 0.0%
AST.NumLinkLibraries 0 0 0 0.0%
AST.NumLoadedModules 127,556 127,556 0 0.0%
AST.NumLocalTypeDecls 5 5 0 0.0%
AST.NumObjCMethods 13,266 13,266 0 0.0%
AST.NumPostfixOperators 14 14 0 0.0%
AST.NumPrecedenceGroups 10,001 10,001 0 0.0%
AST.NumPrefixOperators 74 74 0 0.0%
AST.NumReferencedDynamicNames 152 152 0 0.0%
AST.NumReferencedMemberNames 2,375,412 2,375,544 132 0.01%
AST.NumReferencedTopLevelNames 166,584 166,584 0 0.0%
AST.NumSourceBuffers 226,745 226,745 0 0.0%
AST.NumSourceLines 1,565,011 1,565,011 0 0.0%
AST.NumSourceLinesPerSecond 1,049,004 1,042,580 -6,424 -0.61%
AST.NumTotalClangImportedEntities 2,968,894 2,969,074 180 0.01%
AST.NumUsedConformances 121,419 121,411 -8 -0.01%
Driver.ChildrenMaxRSS 42,749,181,952 42,849,310,720 100,128,768 0.23%
Driver.DriverDepCascadingDynamic 0 0 0 0.0%
Driver.DriverDepCascadingExternal 0 0 0 0.0%
Driver.DriverDepCascadingMember 0 0 0 0.0%
Driver.DriverDepCascadingNominal 0 0 0 0.0%
Driver.DriverDepCascadingTopLevel 0 0 0 0.0%
Driver.DriverDepDynamic 0 0 0 0.0%
Driver.DriverDepExternal 0 0 0 0.0%
Driver.DriverDepMember 0 0 0 0.0%
Driver.DriverDepNominal 0 0 0 0.0%
Driver.DriverDepTopLevel 0 0 0 0.0%
Driver.NumDriverJobsRun 10,271 10,271 0 0.0%
Driver.NumDriverJobsSkipped 0 0 0 0.0%
Driver.NumDriverPipePolls 143,358 142,200 -1,158 -0.81%
Driver.NumDriverPipeReads 155,758 155,336 -422 -0.27%
Driver.NumProcessFailures 0 0 0 0.0%
Frontend.NumProcessFailures 0 0 0 0.0%
IRModule.NumIRAliases 8,826 8,826 0 0.0%
IRModule.NumIRBasicBlocks 2,358,906 2,358,944 38 0.0%
IRModule.NumIRComdatSymbols 0 0 0 0.0%
IRModule.NumIRFunctions 1,348,636 1,348,674 38 0.0%
IRModule.NumIRGlobals 1,586,306 1,586,409 103 0.01%
IRModule.NumIRIFuncs 0 0 0 0.0%
IRModule.NumIRInsts 28,578,720 28,579,455 735 0.0%
IRModule.NumIRNamedMetaData 50,810 50,810 0 0.0%
IRModule.NumIRValueSymbols 2,536,357 2,536,460 103 0.0%
LLVM.NumLLVMBytesOutput 721,792,372 721,812,492 20,120 0.0%
Parse.NumFunctionsParsed 92,156 92,156 0 0.0%
SILModule.NumSILGenDefaultWitnessTables 0 0 0 0.0%
SILModule.NumSILGenFunctions 1,201,288 1,201,497 209 0.02%
SILModule.NumSILGenGlobalVariables 14,815 14,815 0 0.0%
SILModule.NumSILGenVtables 9,738 9,738 0 0.0%
SILModule.NumSILGenWitnessTables 25,788 25,788 0 0.0%
SILModule.NumSILOptDefaultWitnessTables 0 0 0 0.0%
SILModule.NumSILOptFunctions 936,740 936,778 38 0.0%
SILModule.NumSILOptGlobalVariables 15,313 15,313 0 0.0%
SILModule.NumSILOptVtables 15,609 15,609 0 0.0%
SILModule.NumSILOptWitnessTables 46,540 46,540 0 0.0%
Sema.AccessLevelRequest 1,195,190 1,195,190 0 0.0%
Sema.DefaultAndMaxAccessLevelRequest 34,925 34,925 0 0.0%
Sema.EnumRawTypeRequest 9,304 9,304 0 0.0%
Sema.InheritedTypeRequest 57,964 57,964 0 0.0%
Sema.NamedLazyMemberLoadFailureCount 16,107 16,107 0 0.0%
Sema.NumConformancesDeserialized 2,968,759 2,974,817 6,058 0.2%
Sema.NumDeclsDeserialized 20,818,985 20,830,137 11,152 0.05%
Sema.NumFunctionsTypechecked 596,407 596,578 171 0.03%
Sema.NumGenericSignatureBuilders 1,028,734 1,028,832 98 0.01%
Sema.NumLazyGenericEnvironments 3,943,766 3,944,869 1,103 0.03%
Sema.NumLazyGenericEnvironmentsLoaded 444,590 445,122 532 0.12%
Sema.NumLazyIterableDeclContexts 3,488,513 3,492,360 3,847 0.11%
Sema.NumTypesDeserialized 21,892,396 21,898,187 5,791 0.03%
Sema.NumTypesValidated 1,796,692 1,796,604 -88 -0.0%
Sema.NumUnloadedLazyIterableDeclContexts 2,366,366 2,370,104 3,738 0.16%
Sema.SetterAccessLevelRequest 69,264 69,264 0 0.0%
Sema.SuperclassTypeRequest 67,922 67,922 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) (2)
name old new delta delta_pct
LLVM.NumLLVMBytesOutput 674,635,932 674,634,040 -1,892 -0.0%
time.swift-driver.wall 2356.0s 2364.9s 8.9s 0.38%

release detailed

Regressed (1)
name old new delta delta_pct
Sema.NumConstraintScopes 6,545,178 7,146,047 600,869 9.18% ⛔
Improved (1)
name old new delta delta_pct
Sema.NumDeclsValidated 1,297,976 1,271,298 -26,678 -2.06% ✅
Unchanged (delta < 1.0% or delta < 100.0ms) (21)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 219,794 220,019 225 0.1%
AST.NumLoadedModules 13,022 13,022 0 0.0%
AST.NumTotalClangImportedEntities 705,597 705,642 45 0.01%
AST.NumUsedConformances 126,234 126,226 -8 -0.01%
IRModule.NumIRBasicBlocks 1,955,920 1,955,920 0 0.0%
IRModule.NumIRFunctions 1,079,236 1,079,236 0 0.0%
IRModule.NumIRGlobals 1,255,189 1,255,177 -12 -0.0%
IRModule.NumIRInsts 19,237,447 19,237,415 -32 -0.0%
IRModule.NumIRValueSymbols 2,111,718 2,111,706 -12 -0.0%
LLVM.NumLLVMBytesOutput 674,635,932 674,634,040 -1,892 -0.0%
SILModule.NumSILGenFunctions 478,216 478,271 55 0.01%
SILModule.NumSILOptFunctions 619,755 619,755 0 0.0%
Sema.NumConformancesDeserialized 1,428,044 1,431,348 3,304 0.23%
Sema.NumDeclsDeserialized 4,631,958 4,640,398 8,440 0.18%
Sema.NumFunctionsTypechecked 270,734 270,869 135 0.05%
Sema.NumGenericSignatureBuilders 237,308 237,414 106 0.04%
Sema.NumLazyGenericEnvironments 806,948 806,855 -93 -0.01%
Sema.NumLazyGenericEnvironmentsLoaded 101,728 101,943 215 0.21%
Sema.NumLazyIterableDeclContexts 568,362 569,832 1,470 0.26%
Sema.NumTypesDeserialized 5,586,754 5,591,848 5,094 0.09%
Sema.NumTypesValidated 825,844 825,752 -92 -0.01%

@rudkx
Copy link
Contributor Author

rudkx commented Jul 13, 2018

The source compatibility suite failures are unrelated to my changes and are all happening on the builders.

@rudkx rudkx requested a review from xedin July 13, 2018 11:10
xedin
xedin reviewed Jul 13, 2018
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.

Changes looks good to me, but from souce compatibility perspective Kitura and SPR now both fail with “too complex” expressions...

@rudkx
Copy link
Contributor Author

rudkx commented Jul 13, 2018

@xedin Ah, I missed that in all the noise, and was waiting on swiftlang/swift-source-compat-suite#218 before re-running.

I'll take a look.

@rudkx
Copy link
Contributor Author

rudkx commented Jul 17, 2018

@swift-ci Please test source compatibility

@rudkx
Copy link
Contributor Author

rudkx commented Sep 21, 2018

It's easy enough to rip this out when the time comes. There are conflicts with this PR, so I'll close and re-do later.

@rudkx rudkx closed this Sep 21, 2018
@rudkx rudkx deleted the remove-shrink-from-solver branch October 21, 2018 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xedin xedin xedin left review comments

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
@rudkx @swift-ci @xedin