-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Do not type check lazy accessor bodies eagerly. #3354
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
Conversation
@swift-ci Please test. |
@DougGregor Doug, can you take a look at this small change to remove some of the cross-file type checking overhead we're seeing in project builds? |
Woo, thank you for working on this @rudkx! |
Awesome @rudkx! Do you think this optimization should apply to all accessors, not just those for lazy properties? Obviously stored property accessors do not have bodies so typeCheckDecl() does very little work there, but at least for computed properties this might help. I still see a lot of explicit calls to typeCheckDecl() on accessors, but maybe the remaining ones are actually needed. |
@swift-ci Please test. |
@slavapestov Yes, there's definitely more to do here, but I wanted to knock off this piece first, and address the other issues separately. Addressing some things will probably be straightforward like this, but others definitely look like they will require some more extensive reworking. |
Can you add a test case for this, to be really sure it's resolved? I'm concerned it's not the accessors that are actually causing the slowdown but the initial value expression itself. |
@jrose-apple I've been thinking about how to test this, along with other type checking performance issues. I think in time we should build a separate type checker performance suite that can be run on the side, e.g. by a bot. I thought about using -debug-time-function-bodies and FileChecking the output to see which functions get checked, but I noticed that depending on how/where exactly the expression type checker gets run, I've found that the time may get charged to the accessor, the first function that results in the type itself getting checked, or to no function at all (in the case of the initializers of some pattern binding decls). Overall this seems pretty fragile. Other ideas? On the specific concern you have, yes, in general we're paying a high cost for non-typed patterns with initializers, type checking them for every file the type appears in. One of the further improvements I want to look at is avoiding that cost when we can (e.g. when we don't need to generate the layout for the type, and when that particular initialized value is not used). From what I saw we don't do that type checking for typed-patterns though (at least for lazy, I need to verify for non-lazy), and I verified in this particular case that the cost was coming from type checking the lazy getter (which has the initializer baked into it). |
Thanks for the explanation. -debug-time-function-bodies really does just time function bodies, so you could probably force it to explicitly output a check line by using the common "wrap in a closure for multi-statement initialization" idiom: lazy var foo: Int = {
var result = 1
result += 1
return result
}() Because it's a multi-statement closure, its time should never be folded into any other check. |
This looks great. Thanks! |
Start chipping away at cross-file type checker performance issues by avoiding type checking of lazy property accessor bodies when the type in question is defined in a different file and the lazy property is a typed pattern. We still type check these in the file they are defined in when we go to type check the types defined within that file. Resolves rdar://problem/26894118.
It turns out the type checker already had a debug option to ensure we don't descend into type checking things we don't want to type check, so I've updated an existing test for this case and verified that it fails without my change and passes with it. |
@swift-ci Smoke test and merge. |
Huzzah, thanks again @rudkx! |
Hello, I was researching on why the lazy accessor get so long to be compiled. Am I safe to assume that if I somehow define a lazy accessor with a type A that located in different file, but if I somehow return a type C inside the body, the compiler will allows it, or I fundamentally misunderstood something? |
The problem was that we were type-checking the initializer of the lazy variable from all of the other files, even when the variable had a type annotation. If the initializer was large (say, a big closure), that could be an overall compile-time performance problem. |
…lang#3354) `pip` is not always installed on macOS, and `pip3` should be enough to build. Let's make the use of `pip` conditional.
What's in this pull request?
Remove some cross-file type checking overhead of lazy property accessors of properties that are typed patterns.
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.
Start chipping away at cross-file type checker performance issues by
avoiding type checking of lazy property accessor bodies when the type in
question is defined in a different file and the lazy property is a typed
pattern.
We still type check these in the file they are defined in when we go to
type check the types defined within that file.
Resolves rdar://problem/26894118.