swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.61k stars 10.37k forks source link

[SR-6727] Instance variables are type checked many times per build #49276

Open swift-ci opened 6 years ago

swift-ci commented 6 years ago
Previous ID SR-6727
Radar None
Original Reporter plivesey (JIRA User)
Type Bug

Attachment: Download

Environment Xcode Version 9.2 (9C40b) Building for device
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 6eb62df3bb41af55053d68fc01464c75

Issue Description:

If you don't include a type annotation for instance variables, then the compiler type checks them once (or twice?) per file in the project. This means for large projects, it will type check the same expression many times causing it to slow down.

I have a sample project which demonstrates this, but for our project, I found that some very simple lines like:

var x = ""

were taking 3.5 seconds total to compile. This is because it was getting type checked hundreds of times (2x our total number of files). If I changed it to:

var x: String = ""

it would only type check once and would take \~1ms.

I went through all our model files (\~15/281 files) and added type annotations (note: these are all VERY simple expressions like empty strings and setting variables to 0). This decreased compile times by nearly 10%. It seems like these types can be cached so users don't need to add type annotations to every variable in their project.

Sample Project

I have a sample project which demonstrates the problem. Since it's so small, adding a type annotation doesn't make a noticeable difference, but using https://github.com/RobertGummesson/BuildTimeAnalyzer-for-Xcode you can clearly see the extra occurances of the type checker on this line.

Reproduce

1. Do a clean build of the project and check the build time analyzer

2. Switch the var in ViewController to use a specific type

3. Build again and check the build time analyzer

Expected

Build outputs are almost identical

Actual

The var is compiled 22 times the first run and only 2 times on the second run.

Notes

Again, this doesn't make a big difference in this tiny project, but for a bigger project with many files and many variables, it can really slow down compilation. It seems like the compiler should be able to do this instead of manually adding these types to all our instance vars.

Another interesting thing is that it sometimes doesn't make much of a difference if you just convert one instance var in a file. You need to do every single instance var before it makes a big difference

swift-ci commented 6 years ago

Comment by Peter Livesey (JIRA)

i just finished converting our whole project. It sped up time for the main module by 25%. None of these expressions took a long time on their own (they were all super simple), and it's definitely a very small part of our code by lines, but it made a huge difference.

belkadan commented 6 years ago

cc graydon (JIRA User)

belkadan commented 6 years ago

That's a cost of running separate jobs to compile separate files, but yes.

swift-ci commented 6 years ago

Comment by Peter Livesey (JIRA)

Is there an architecture reason why this isn't cached? It seems like you could type check a file once and store it somewhere?

But presume there's some reason that's not done?

belkadan commented 6 years ago

It's not currently possible to know that the set of changes in a project didn't affect the type of an inferred-type stored variable. docs/Driver.md in the Swift repo has some information about this.

typealias Foo = Bar
struct Bar {
  static func baz() -> Int { return 42 }
}
let constant = Foo.baz()
belkadan commented 6 years ago

Honestly I'd rather just push for saying "only literals can be used for non-file-private stored variables without explicit types" and make that case really fast than come up with a way to solve this specific problem.

swift-ci commented 6 years ago

Comment by Peter Livesey (JIRA)

I may be missing something here, but in this case, the type of constant would be Int in all three files right? There's never a case where constant isn't an Int. So it shouldn't need to be type checked 3 times (one for each file), but just once?

belkadan commented 6 years ago

It's an incremental build problem. If you change B.swift, the compiler knows it also needs to recompile A.swift, but it might not know it needs to recompile C.swift until it finishes with A.swift. In that case, it's possible the cached information from C.swift is invalid, and if A.swift depended on that information we'd need to compile it again.

(This particular case doesn't actually have that problem; it needs to be more contrived. But such cases exist and we have tests for them.)

swift-ci commented 6 years ago

Comment by Peter Livesey (JIRA)

Reading through those docs. Very useful thanks.

It seems like this is a key line:

"A job is only responsible for compiling its primary file, and only does as much work as it needs to compile that file, lazily type-checking declarations in other files in the module."

I presume that lazily type checking doesn't mean these results are shared between jobs. It doesn't seem like any type checking information is shared between jobs.

But, in this bug, all of the properties in the project were compiled 183 times - even though those properties weren't used in most files. It seems like it's possible that instance vars are getting type checked not lazily?

swift-ci commented 6 years ago

Comment by Peter Livesey (JIRA)

Sorry, posted that comment before refreshing the page, so missed your last answer.

That does make sense re: caching. If you turn on caching, then it makes incremental builds much harder to reason about (leading to bugs).

swift-ci commented 6 years ago

Comment by Peter Livesey (JIRA)

Maybe one problem is that my sample project is really contrived. We have a bunch of leaf files which do very generic things (like add an extension to array or something). It 'looks' like when we compile this file, it's still type checking all the ivars in the project.

I'm using a dependency graph visualizer and see that many files don't have any dependencies (just 1 or 2 files that depend on it).

Btw, my assumption about ivars getting type checked for every job is because the occurrences == number of swift files and is the same for most of the ivars in the project (but not actually all of them).