File Hashing for Incremental Builds #1923

benb · 2025-06-11T11:09:38Z

This PR:

Adds a command line option --enable-incremental-file-hashing, and a corresponding --disable-incremental-file-hashing, defaulting to disabled
When file-hashing is enabled, both the source files and external dependency files are hashed with SHA256
Those hashes are used to verify that a file is unchanged even if the timestamp is updated
They are serialized with the rest of the ModuleDependencyGraph info in the blob field
The serialized version is given a minor version bump to account for this change
Tests added

RFC discussion here: https://forums.swift.org/t/rfc-file-hashes-for-swift-incremental-compilation/

drodriguez · 2025-06-11T13:36:58Z

@swift-ci please smoke test

Broken tree state because swiftlang/swift#82153 had not landed yet.

drodriguez · 2025-06-11T15:16:06Z

@swift-ci please smoke test

owenv

LGTM, just had a few minor comments

owenv · 2025-06-11T16:09:23Z

Sources/SwiftDriver/Driver/Driver.swift

@@ -44,6 +45,14 @@ extension Driver.ErrorDiagnostics: CustomStringConvertible {
  }
 }

+public struct FileMetadata {


Could this be marked @_spi(Testing)? I don't think it needs to be API

I'm afraid this needs to be exposed in the DriverExecutor protocol, so I think it needs to be public.

Ah, I forgot this showed up in that API. We may need a strategy to stage in the changes to the DriverExecutor protocol so they don't break https://github.com/swiftlang/swift-build, or make synchronized changes to the repos. Maybe you could keep the old execute requirements around temporarily for default implementations of the new ones to call?

Ah, I didn't realise there was an API here exposed to swift-build.

I've added a commit that restores the original methods to DriverExecutor, with an extension that gives default implementations of the new methods, and added in definitions with fatalError() if these legacy methods are called on the implementations defined in swift-driver itself.

3ecc734

I prefer this to move to a file in SwiftDriver/Utilities/. Remember to update the CMakeFile if you add a new file.

Sources/SwiftDriver/Driver/Driver.swift

benb · 2025-06-12T13:52:13Z

I had to fix the added tests - the old logic stopped working after rebasing this branch past 7c709c5

Honestly, I'm not sure my tests are the right approach, however the majority of tests in IncrementalCompilationTests are failing for me locally on main, so I don't want to copy those approaches directly. Maybe I am doing something wrong locally? Most other tests are passing for me, though.

artemcm · 2025-06-13T16:42:39Z

however the majority of tests in IncrementalCompilationTests are failing for me locally on main, so I don't want to copy those approaches directly.

What kinds of failures are you seeing? I recommend building and running tests against most-recent main snapshot from https://www.swift.org/install/macos/ or a locally-built ToT main of the compiler.

benb · 2025-06-13T16:51:26Z

however the majority of tests in IncrementalCompilationTests are failing for me locally on main, so I don't want to copy those approaches directly.

What kinds of failures are you seeing? I recommend building and running tests against most-recent main snapshot from https://www.swift.org/install/macos/ or a locally-built ToT main of the compiler.

Test Suite 'IncrementalCompilationTests' failed at 2025-06-13 17:30:25.064. Executed 37 tests, with 31 failures (0 unexpected) in 49.195 (49.197) seconds

I'll try with a snapshot.

benb · 2025-06-13T18:52:18Z

I'll try with a snapshot.
Using swiftly main-snapshot on a commit from yesterday:

$ ~/.swiftly/bin/swift --version
Apple Swift version 6.2-dev (LLVM d266fd9f80a7945, Swift 665515c781999a8)
Target: arm64-apple-macosx15.0
Build config: +assertions

I'm still getting lots of failures in IncrementalCompilationTests on current main (8c130e3):

$ ~/.swiftly/bin/swift test
…
Test Suite 'IncrementalCompilationTests' failed at 2025-06-13 19:33:51.708.
         Executed 34 tests, with 31 failures (0 unexpected) in 132.160 (132.161) seconds
…

Although I see strictly speaking there are '0 unexpected' failures, but I'm not sure how or why these are defined as being 'expected'.

benb · 2025-06-20T16:18:31Z

@owenv Is there anything I'm missing to unblock this PR?

owenv · 2025-06-20T16:22:26Z

No, sorry, I lost track of this. Let's kick off the CI tests and see how those results look.

@swift-ci test

owenv · 2025-06-20T16:40:25Z

@swift-ci test Windows

benb · 2025-06-23T15:59:58Z

Rebased and squashed fixup commits.

@swift-ci test
@swift-ci test Windows

owenv · 2025-06-23T16:06:19Z

@swift-ci test

artemcm · 2025-06-23T16:17:28Z

@swift-ci test Windows platform

cachemeifyoucan

There are some incremental state reporter messages need to be adjusted, or report a different message. Like reportExplicitDependencyOutOfDate says older than input file.

The logic changes are good to me.

cachemeifyoucan · 2025-06-23T16:16:32Z

Sources/SwiftDriver/Driver/Driver.swift

@@ -44,6 +45,14 @@ extension Driver.ErrorDiagnostics: CustomStringConvertible {
  }
 }

+public struct FileMetadata {


I prefer this to move to a file in SwiftDriver/Utilities/. Remember to update the CMakeFile if you add a new file.

benb · 2025-06-24T15:29:23Z

There are some incremental state reporter messages need to be adjusted, or report a different message. Like reportExplicitDependencyOutOfDate says older than input file.

This message is still accurate, alas. I haven't added hash logic here, and I don't think this PR can help in this case without adding a lot more work.

I may have understood this wrong, but I think in this case we are comparing swiftmodule files that are dependencies or transitive dependencies of a file under compilation in the current module ("Module A"). And if the timestamp of a dependency-of-a-dependency ("Module C") is newer than that of the dependency ("Module B"), this is taken as meaning we have to recompile Module B before we can proceed. Of course, if the hash of Module C is unchanged, we could skip it.

However, I don't think there's an easy place to store the hash data for these (and we are not storing timestamps for them). Furthermore, I think you would want to compare the hash of Module C with that recorded during the compilation of Module B, not during the previous compilation of the current module (Module A).

I also think this logic is invoked during WMO compilation, and not just during Incremental mode.

Does this sound right?

cachemeifyoucan · 2025-06-24T16:38:44Z

This message is still accurate, alas. I haven't added hash logic here, and I don't think this PR can help in this case without adding a lot more work.

Ah, this is the report for module dependency only and I guess we are not extending hash check to module dependencies yet (and that is not computed in swift driver, but in scanner). The remark for incremental build only says skipped without stating if the file changed or it is older, so I guess it is fine.

Thanks for looking and address the feedback. I think the newly added FileMetadata.swift should be in Sources/SwiftDriver/Utilities not in its current location where there are no other swift source files. LGTM otherwise.

benb · 2025-06-24T16:43:45Z

I think the newly added FileMetadata.swift should be in Sources/SwiftDriver/Utilities not in its current location where there are no other swift source files.

Whoops, fixed. Thanks for your review.

benb · 2025-06-24T17:09:45Z

Can someone with the power run the required swift-ci checks again, please?

artemcm · 2025-06-24T17:10:36Z

@swift-ci test

artemcm · 2025-06-24T17:10:59Z

@swift-ci test Windows platform

cachemeifyoucan · 2025-06-24T17:21:16Z

@swift-ci test Windows platform

benb · 2025-06-24T19:12:32Z

Rebased past b0a01d3 to fix merge conflicts, now we need to run the CI tests again…

artemcm · 2025-06-24T19:46:40Z

@swift-ci test

artemcm · 2025-06-24T19:47:01Z

@swift-ci test Windows platform

artemcm · 2025-06-24T19:47:21Z

Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift

@@ -536,7 +536,7 @@ final class ExplicitModuleBuildTests: XCTestCase {
                  main.nativePathString(escaped: true)] + sdkArgumentsForTesting
      var driver = try Driver(args: args)
      let _ = try driver.planBuild()
-      let dependencyGraph = try XCTUnwrap(driver.explicitDependencyBuildPlanner?.dependencyGraph)
+      let dependencyGraph = try XCTUnwrap(driver.intermoduleDependencyGraph)


benb · 2025-06-25T07:35:52Z

I can't see the merge button - does a committer need to merge on my behalf?

owenv reviewed Jun 11, 2025

View reviewed changes

owenv approved these changes Jun 20, 2025

View reviewed changes

benb force-pushed the incremental-hash branch from 3ecc734 to 28aa644 Compare June 23, 2025 15:58

artemcm approved these changes Jun 23, 2025

View reviewed changes

cachemeifyoucan reviewed Jun 23, 2025

View reviewed changes

benb force-pushed the incremental-hash branch from 28aa644 to b1d9967 Compare June 24, 2025 15:45

benb force-pushed the incremental-hash branch from b1d9967 to 4631ba4 Compare June 24, 2025 16:42

cachemeifyoucan approved these changes Jun 24, 2025

View reviewed changes

benb added 3 commits June 24, 2025 20:09

Fix test build

96f2b42

Add incremental hash option to swift-driver

87c242c

source file hashing for incremental

87d7e50

benb added 2 commits June 24, 2025 20:09

external dependency hashing for incremental

3e2f3ca

Bump serialized version

d1c7f96

benb force-pushed the incremental-hash branch from b1f3220 to d1c7f96 Compare June 24, 2025 19:11

artemcm reviewed Jun 24, 2025

View reviewed changes

drodriguez merged commit a7f32bd into swiftlang:main Jun 25, 2025
3 checks passed

benb mentioned this pull request Jun 26, 2025

Add incremental-file-hashing options to swift-driver swiftlang/swift#82525

Merged

File Hashing for Incremental Builds #1923

File Hashing for Incremental Builds #1923

Uh oh!

Conversation

benb commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drodriguez commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drodriguez commented Jun 11, 2025

Uh oh!

owenv left a comment

Choose a reason for hiding this comment

Uh oh!

owenv Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

benb Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

owenv Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

benb Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

cachemeifyoucan Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

benb commented Jun 12, 2025

Uh oh!

artemcm commented Jun 13, 2025

Uh oh!

benb commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benb commented Jun 13, 2025

Uh oh!

benb commented Jun 20, 2025

Uh oh!

owenv commented Jun 20, 2025

Uh oh!

owenv commented Jun 20, 2025

Uh oh!

benb commented Jun 23, 2025

Uh oh!

owenv commented Jun 23, 2025

Uh oh!

artemcm commented Jun 23, 2025

Uh oh!

cachemeifyoucan left a comment

Choose a reason for hiding this comment

Uh oh!

cachemeifyoucan Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

benb commented Jun 24, 2025

Uh oh!

cachemeifyoucan commented Jun 24, 2025

Uh oh!

benb commented Jun 24, 2025

Uh oh!

benb commented Jun 24, 2025

Uh oh!

artemcm commented Jun 24, 2025

Uh oh!

artemcm commented Jun 24, 2025

Uh oh!

cachemeifyoucan commented Jun 24, 2025

Uh oh!

benb commented Jun 24, 2025

Uh oh!

artemcm commented Jun 24, 2025

Uh oh!

artemcm commented Jun 24, 2025

Uh oh!

artemcm Jun 24, 2025

benb commented Jun 11, 2025 •

edited

Loading

drodriguez commented Jun 11, 2025 •

edited

Loading

benb commented Jun 13, 2025 •

edited

Loading