[Commands] Remove build manifest caching

<rdar://problem/42583418> Remove build manifest caching

The build manifest caching provides a preformance win but creating build
plan is now faster (than build manifest caching) because of package
manifest caching. The difference is about 100ms and we can bring the
time down by optimizing the tools lookup. However, this feature is not
critical anymore because the performance is decent with package manifest
caching. We can add it back in the future when required.


Author: aciidgh

Sources/Commands/SwiftBuildTool.swift

@@ -53,16 +53,9 @@ public class SwiftBuildTool: SwiftTool<BuildToolOptions> {
 
             guard let subset = options.buildSubset(diagnostics: diagnostics) else { return }
 
-            // Check if we need to generate the llbuild manifest.
-            let regenerateManifest = try shouldRegenerateManifest()
-            if regenerateManifest {
-                // Create the build plan and build normally.
-                let plan = try BuildPlan(buildParameters: buildParameters(), graph: loadPackageGraph(), diagnostics: diagnostics)
-                try build(plan: plan, subset: subset)
-            } else {
-                // Otherwise, run llbuild directly.
-                try build(parameters: buildParameters(), subset: subset)
-            }
+           // Create the build plan and build.
+           let plan = try BuildPlan(buildParameters: buildParameters(), graph: loadPackageGraph(), diagnostics: diagnostics)
+           try build(plan: plan, subset: subset)
 
         case .binPath:
             try print(buildParameters().buildPath.asString)

Sources/Commands/SwiftTool.swift

@@ -374,11 +374,6 @@ public class SwiftTool<Options: ToolOptions> {
                 usage: "Link Swift stdlib statically"),
             to: { $0.shouldLinkStaticSwiftStdlib = $1 })
 
-        binder.bind(
-            option: parser.add(option: "--enable-build-manifest-caching", kind: Bool.self,
-                usage: "Enable llbuild manifest caching [Experimental]"),
-            to: { $0.shouldEnableManifestCaching = $1 })
-
         // Let subclasses bind arguments.
         type(of: self).defineArguments(parser: parser, binder: binder)
 
@@ -539,13 +534,10 @@ public class SwiftTool<Options: ToolOptions> {
             // Throw if there were errors when loading the graph.
             // The actual errors will be printed before exiting.
             guard !diagnostics.hasErrors else {
-                try buildManifestRegenerationToken().set(valid: false)
                 throw Diagnostics.fatalError
             }
-            try buildManifestRegenerationToken().set(valid: true)
             return graph
         } catch {
-            try buildManifestRegenerationToken().set(valid: false)
             throw error
         }
     }
@@ -559,30 +551,6 @@ public class SwiftTool<Options: ToolOptions> {
         return try _manifestLoader.dematerialize()
     }
 
-    func shouldRegenerateManifest() throws -> Bool {
-        // Check if we are allowed to use llbuild manifest caching.
-        guard options.shouldEnableManifestCaching else {
-            return true
-        }
-        
-        // Check if we need to generate the llbuild manifest.
-        let parameters: BuildParameters = try self.buildParameters()
-        guard localFileSystem.isFile(parameters.llbuildManifest) else {
-            return true
-        }
-        
-        // Run the target which computes if regeneration is needed.
-        let args = [try getToolchain().llbuild.asString, "-f", parameters.llbuildManifest.asString, "regenerate"]
-        do {
-            try Process.checkNonZeroExit(arguments: args)
-        } catch {
-            // Regenerate the manifest if this fails for some reason.
-            warning(message: "Failed to run the regeneration check: \(error)")
-            return true
-        }
-        return try !self.buildManifestRegenerationToken().isValid()
-    }
-    
     func computeLLBuildTargetName(for subset: BuildSubset) throws -> String? {
         switch subset {
         case .allExcludingTests:
@@ -711,7 +679,6 @@ public class SwiftTool<Options: ToolOptions> {
                 destinationTriple: triple,
                 flags: options.buildFlags,
                 shouldLinkStaticSwiftStdlib: options.shouldLinkStaticSwiftStdlib,
-                shouldEnableManifestCaching: options.shouldEnableManifestCaching,
                 sanitizers: options.sanitizers
             )
         })

Tests/CommandsTests/BuildToolTests.swift

@@ -177,30 +177,4 @@ final class BuildToolTests: XCTestCase {
             }
         }
     }
-
-    func testLLBuildManifestCachingBasics() {
-        fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { path in
-            let fs = localFileSystem
-
-            // First run should produce output.
-            var output = try execute(["--enable-build-manifest-caching"], packagePath: path)
-            XCTAssert(!output.isEmpty, output)
-
-            // Null builds.
-            output = try execute(["--enable-build-manifest-caching"], packagePath: path)
-            XCTAssert(output.isEmpty, output)
-
-            output = try execute(["--enable-build-manifest-caching"], packagePath: path)
-            XCTAssert(output.isEmpty, output)
-
-            // Adding a new file should reset the token.
-            try fs.writeFileContents(path.appending(components: "Sources", "ExecutableNew", "bar.swift"), bytes: "")
-            output = try execute(["--enable-build-manifest-caching"], packagePath: path)
-            XCTAssert(!output.isEmpty, output)
-
-            // This should be another null build.
-            output = try execute(["--enable-build-manifest-caching"], packagePath: path)
-            XCTAssert(output.isEmpty, output)
-        }
-    }
 }

Tests/CommandsTests/XCTestManifests.swift

@@ -4,7 +4,6 @@ import XCTest
 extension BuildToolTests {
     static let __allTests = [
         ("testBinPathAndSymlink", testBinPathAndSymlink),
-        ("testLLBuildManifestCachingBasics", testLLBuildManifestCachingBasics),
         ("testNonReachableProductsAndTargetsFunctional", testNonReachableProductsAndTargetsFunctional),
         ("testProductAndTarget", testProductAndTarget),
         ("testSeeAlso", testSeeAlso),