[SR-12345] Swift Driver Outputs Invalid Commands For Incremental Builds

[swift-ci]

Previous ID	SR-12345
Radar	rdar://problem/60832292
Original Reporter	milen (JIRA User)
Type	Bug
Status	Resolved
Resolution	Won't Do

Attachment: Download

Environment

• Apple Swift version 5.1.3 (swiftlang-1100.0.282.1 clang-1100.0.33.15)

• Target: x86_64-apple-darwin18.7.0

• macOS 10.14.6

Additional Detail from JIRA

Votes	0
Component/s
Labels	Bug
Assignee	None
Priority	Medium

md5: e3a191af65d865ab008c6a0bb2b514d7

Issue Description:

The Problem

When swiftc is asked to output object file commands in incremental mode (i.e., -c, -incremental and -###) with prior state, it does not correctly output all commands necessary to recompile affected files.

Context

The [driver documentation|https://github.com/apple/swift/blob/master/docs/Driver.md] outlines three ways for build systems to compile Swift code:

1. -emit-executable or -emit-library

2. -c to produce object files (can be combined with -incremental)

3. -c -### to emit commands to be executed externally (can be combined with -incremental)

Option 1 is not practical for production build systems (e.g., mixed libraries; control over compilation process).

Option 2 places two important constraints/restrictions on build systems:

• Inability to fully control parallelism across the build process.

  • For example, build systems want to control parallelisation across all jobs.

  • Fine grained control is not possible using just -j / -num-threads.

• Inability to distribute command execution across multiple machines.

  • By design, all compilation will be performed on the local machine.

  • This prohibits the build system distributing jobs over a cluster of machines.

Due to the above, option 3 is the most suitable for scalable build systems.

Documentation

Regarding Option 3, the [documentation|https://github.com/apple/swift/blob/master/docs/Driver.md] states:

This is the most hacky approach, because (a) it involves looking for an internal flag in a non-stable interface, and (b) you don't get anything incremental out of this. We could stand some improvements here.

There are two possibilities here regarding the current behaviour:

• -c -### is incompatible with -incremental by design

  • In that case, the driver should return an error rather than a subset of the appropriate commands.

  • The documentation can be adjusted as well.

• -c -### is compatible with -incremental by design

  • In that case, it's a bug.

Why's That Important

The current bug precludes the usage of Option 3 and restricts external build systems to Option 2. This limits scalability and parallelism control.

Reproducible Steps

Setup

1. Download the attached files in a directory (ROOT_DIR)

Verify Behaviour

In this step, we verify that incremental compilation works correctly.

1. cd $ROOT_DIR

2. rm -rf build

3. mkdir build

4. swiftc -c ConstantUser.swift ConstantHolder.swift -module-name Constants -emit-module -emit-module-path build/Constants.swiftmodule -incremental -output-file-map output.map

5. shasum build/*.o

   1. Let's call these "Initial Values"

6. Open ConstantHolder.swift

   1. Comment out static let SomeValue = 5

   2. Uncomment static let SomeValue = "Hello"

   3. Save file

7. swiftc -c ConstantUser.swift ConstantHolder.swift -module-name Constants -emit-module -emit-module-path build/Constants.swiftmodule -incremental -output-file-map output.map

8. shasum build/*.o

   1. Note that the SHA1 checksums of both files have changed

   2. Let's call these "Final Values"

9. rm -rf build

10. mkdir build

11. swiftc -c ConstantUser.swift ConstantHolder.swift -module-name Constants -emit-module -emit-module-path build/Constants.swiftmodule -output-file-map output.map

    1. NB: There's no -incremental flag

12. shasum build/*.o

    1. Compare values against "Final Values". They're identical.

    2. This demonstrates incremental and non-incremental produce the results.

13. Restore ConstantHolder.swift to its original state

    1. NB: Very important step!

Broken Behaviour

Here, we will demonstrate that the Swift driver will not recompile ConstantUser.o even though it should.

1. cd $ROOT_DIR

2. Ensure that ConstantHolder.swift is restored to its original state

   1. static let SomeValue = 5 should be uncommented

3. rm -rf build

4. mkdir build

5. swiftc -c ConstantUser.swift ConstantHolder.swift -module-name Constants -emit-module -emit-module-path build/Constants.swiftmodule -incremental -output-file-map output.map

6. shasum build/*.o

   1. Note those are identical to "Initial Values" from "Verify Behaviour" section

7. Open ConstantHolder.swift

   1. Comment out static let SomeValue = 5

   2. Uncomment static let SomeValue = "Hello"

   3. Save file

8. swiftc -c ConstantUser.swift ConstantHolder.swift -module-name Constants -emit-module -emit-module-path build/Constants.swiftmodule -incremental -output-file-map output.map -###

   1. BUG: You will notice that the driver will output two commands: one to recompile ConstantHolder.o and the other merge the new Constants.swiftmodule

   2. Execute both commands from the output

9. shasum build/*.o

   1. Note that the SHA1 checksums of just ConstantHolder.o has changed (BUG)

   2. Note that ConstantHolder.o has the same SHA1 hash as in "Final Values" but ConstantUser.o does not

10. swiftc -emit-executable main.swift -I build build/ConstantUser.o build/ConstantHolder.o -o build/main

1. 1. You will get a linker error because ConstantUser.o was not recompiled and it still references a symbol of the wrong type (i.e., before the change)

[beccadax]

@swift-ci create

[slavapestov]

> Inability to fully control parallelism across the build process

The desire for clients to have a better understanding of swift compiler jobs for scheduling purposes is actually one reason we're rewriting the driver in Swift. Turning it into a framework will allow for smarter tooling...

[CodaFi]

Right, that document has a serious disconnect from the current state of affairs when running incremental builds. You present two explanations for the current behavior, and I believe the correct choice given what you observe is

-c -### is incompatible with -incremental *for your use-case*

Observe that if you follow your reproduction steps then actually run the driver by replacing -### with -driver-show-incremental, you will see the piece you're missing here:

Queuing (initial): {compile: ConstantHolder.o <= ConstantHolder.swift}
Queuing (initial): {merge-module: Constants.swiftmodule <= ConstantUser.o ConstantHolder.o}
Queuing because of dependencies discovered later: {compile: ConstantUser.o <= ConstantUser.swift}
    ConstantHolder.swift provides type 'Constants.ConstantHolder'

Those two top lines correspond to the frontend and merge-modules commands that you observed before. The last is the part you're missing: You have to run the incremental build to a fixed point. Now, given that, you would think that executing the compile commands from one -### then asking for -### again until you only see e.g. a single merge-modules command or something would be sufficient, but it appears the driver doesn't report the second-wave correctly. Unfortunately, I'm not the domain expert here.

davidungar (JIRA User)?

[CodaFi]

After speaking with David I believe I have an explanation for the behavior you're seeing. Let's follow all of the steps you have up until step 8. At this point, you would expect that the driver would read the state of the swiftdeps files, notice that there is some extra work to do in the second wave that it never got around to. This would naturally lead you to expect running `-###` to report another frontend compile job for ConstantUser.swift. Repeating this process would allow you to run the build to a fix point and extract all the relevant frontend commands.

But this model is forgetting one additional variable: The driver maintains a record of the state of jobs it has executed, and that build record is not being updated by your build system when you just naively schedule frontend commands. Thus, the second wave command is never seen because the second wave command's status is never recorded by the driver in the build record.

So, this raises the question of what to do about it. Certainly, my answer to the above does not change in light of this information: -### is incompatible with what you're trying to do here, and I don't think it makes sense to go to the effort of making it compatible. You literally cannot discover additional dependencies without running more frontend jobs, and if you're already doing that then there's no point in scheduling them yourself. I will be updating the documentation to account for this.

[CodaFi]

#31353

[CodaFi]

(I really dislike that the best classification I have here is "Won't Do")

Anyhow, in light of the above, and that we cannot extend -### to support this use case, I am resolving this issue. I do hope we come to support better 3rd-party incremental build infrastructure in the future.

[swift-ci]

Comment by David Ungar (JIRA)

Thanks, @CodaFi . That's a very clear explanation!