Merge pull request #1166 from rjmansfield/fix-shared-pch-staleness

Fix shared PCH staleness issues due to missing target ordering.

Author: rjmansfield

Sources/SWBCore/SpecImplementations/Tools/CCompiler.swift

@@ -1611,17 +1611,24 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
             let (precompFile, pchInfoAny) = delegate.createOrReuseSharedNodeWithIdentifier(sharingIdentString) { () -> (any PlannedNode, any Sendable) in
                 // This block is invoked only when we need to create a task to precompile a header.
 
+                // Ordering gate: a workspace level task will wire together all sharing targets' startCompilingNode.
+                let orderingNode = delegate.createVirtualNode("shared-pch-ordering-\(sharingIdentHashValue)")
+
                 // Construct the full path of the precomp file, which includes the hash to make it unique.
                 // FIXME: This needs to actually include a hash code, and it should ideally (for comparison reasons) be the same as Xcode.
                 let precompPath = baseCachePath.join("SharedPrecompiledHeaders").join("\(sharingIdentHashValue)").join(prefixHeader.basename + ".gch")
 
                 // Start by invoking our logic to create a task to precompile the prefix header.
-                let pchInfo = self.precompile(cbc, delegate, headerPath: prefixHeader, language: language, inputFileType: inputFileType, extraArgs: perFileFlags, precompPath: precompPath, clangInfo: clangInfo)
+                let pchInfo = self.precompile(cbc, delegate, headerPath: prefixHeader, language: language, inputFileType: inputFileType, extraArgs: perFileFlags, precompPath: precompPath, clangInfo: clangInfo, additionalOrderingInputs: [orderingNode])
 
                 // Return the output node for the precomp file.
                 return (delegate.createNode(precompPath), pchInfo)
             }
 
+            // Ensure shared ProcessPCH depends on all consuming targets upstream headers.
+            let orderingNode = delegate.createVirtualNode("shared-pch-ordering-\(sharingIdentHashValue)")
+            delegate.registerSharedPCHOrderingDependency(sharingIdentString, orderingNode: orderingNode)
+
             let pchInfo = pchInfoAny as! ClangPrefixInfo.PCHInfo
 
             // Include the precomp file.
@@ -1659,7 +1666,7 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
     }
 
     /// Specialized function that creates a task for precompiling a particular header.
-    private func precompile(_ cbc: CommandBuildContext, _ delegate: any TaskGenerationDelegate, headerPath: Path, language: GCCCompatibleLanguageDialect, inputFileType: FileTypeSpec, extraArgs: [String], precompPath: Path, clangInfo: DiscoveredClangToolSpecInfo?) -> ClangPrefixInfo.PCHInfo {
+    private func precompile(_ cbc: CommandBuildContext, _ delegate: any TaskGenerationDelegate, headerPath: Path, language: GCCCompatibleLanguageDialect, inputFileType: FileTypeSpec, extraArgs: [String], precompPath: Path, clangInfo: DiscoveredClangToolSpecInfo?, additionalOrderingInputs: [any PlannedNode] = []) -> ClangPrefixInfo.PCHInfo {
 
         // FIXME: Disabled for now because some projects manages to end up getting here with multiple files. <rdar://problem/23682348> Project groups multiple .c files together for C compiler?
         //let input = cbc.input
@@ -1773,7 +1780,8 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
         // If explicit modules are enabled we need to create the scan task first
         let extraInputs: [Path]
         if action != nil {
-            delegate.createTask(type: self, payload: payload, ruleInfo: ["ScanDependencies"] + ruleInfo.dropFirst(), additionalSignatureData: additionalSignatureData, commandLine: ["builtin-ScanDependencies", "-o", scanningOutput.str, "--"] + commandLine, additionalOutput: responseFileAdditionalOutput, environment: environmentBindings, workingDirectory: compilerWorkingDirectory(cbc), inputs: inputPaths, outputs: [scanningOutput], action: delegate.taskActionCreationDelegate.createClangScanTaskAction(), execDescription: "Scan dependencies of \(headerPath.basename)", preparesForIndexing: true, enableSandboxing: enableSandboxing, additionalTaskOrderingOptions: [.compilationForIndexableSourceFile], usesExecutionInputs: usesExecutionInputs)
+            let scanInputNodes: [any PlannedNode] = inputPaths.map(delegate.createNode) + additionalOrderingInputs
+            delegate.createTask(type: self, payload: payload, ruleInfo: ["ScanDependencies"] + ruleInfo.dropFirst(), additionalSignatureData: additionalSignatureData, commandLine: ["builtin-ScanDependencies", "-o", scanningOutput.str, "--"] + commandLine, additionalOutput: responseFileAdditionalOutput, environment: environmentBindings, workingDirectory: compilerWorkingDirectory(cbc), inputs: scanInputNodes, outputs: [delegate.createNode(scanningOutput)], action: delegate.taskActionCreationDelegate.createClangScanTaskAction(), execDescription: "Scan dependencies of \(headerPath.basename)", preparesForIndexing: true, enableSandboxing: enableSandboxing, additionalTaskOrderingOptions: [.compilationForIndexableSourceFile], usesExecutionInputs: usesExecutionInputs)
 
             extraInputs = [scanningOutput]
         } else {
@@ -1782,7 +1790,8 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
 
         // Finally, create the task.
         let execDescription = archSpecificExecutionDescription(cbc.scope.namespace.parseString("Precompile \(headerPath.basename)"), cbc, delegate)
-        delegate.createTask(type: self, dependencyData: dependencyData, payload: payload, ruleInfo: ruleInfo, additionalSignatureData: additionalSignatureData, commandLine: byteStringCommandLine, additionalOutput: additionalOutput, environment: environmentBindings, workingDirectory: compilerWorkingDirectory(cbc), inputs: inputPaths + extraInputs, outputs: outputPaths, action: action ?? delegate.taskActionCreationDelegate.createDeferredExecutionTaskActionIfRequested(userPreferences: cbc.producer.userPreferences), execDescription: execDescription, enableSandboxing: enableSandboxing, usesExecutionInputs: usesExecutionInputs, showEnvironment: true)
+        let allInputNodes: [any PlannedNode] = (inputPaths + extraInputs).map(delegate.createNode) + additionalOrderingInputs
+        delegate.createTask(type: self, dependencyData: dependencyData, payload: payload, ruleInfo: ruleInfo, additionalSignatureData: additionalSignatureData, commandLine: byteStringCommandLine, additionalOutput: additionalOutput, environment: environmentBindings, workingDirectory: compilerWorkingDirectory(cbc), inputs: allInputNodes, outputs: outputPaths.map(delegate.createNode), mustPrecede: [], action: action ?? delegate.taskActionCreationDelegate.createDeferredExecutionTaskActionIfRequested(userPreferences: cbc.producer.userPreferences), execDescription: execDescription, preparesForIndexing: false, enableSandboxing: enableSandboxing, llbuildControlDisabled: false, additionalTaskOrderingOptions: [], usesExecutionInputs: usesExecutionInputs, showEnvironment: true)
 
         // If the object file verifier is enabled and we are building with explicit modules, also create a job to produce an adjacent PCH using implicit modules.
         if cbc.scope.evaluate(BuiltinMacros.CLANG_ENABLE_EXPLICIT_MODULES_OBJECT_FILE_VERIFIER) && action != nil {

Sources/SWBCore/TaskGeneration.swift

@@ -820,6 +820,9 @@ public protocol TaskGenerationDelegate: AnyObject, TargetDiagnosticProducingDele
     /// FIXME: The name of this method is also a bit awkward.  We could probably do better.
     func createOrReuseSharedNodeWithIdentifier(_ ident: String, creator: () -> (any PlannedNode, any Sendable)) -> (any PlannedNode, any Sendable)
 
+    /// Register the current target as a consumer of the shared PCH.
+    func registerSharedPCHOrderingDependency(_ ident: String, orderingNode: any PlannedNode)
+
     /// Adds the accessed path to the list of paths which invalidate the build description.
     func access(path: Path)
 

Sources/SWBTaskConstruction/ProductPlanning/BuildPlan.swift

@@ -12,6 +12,7 @@
 
 package import SWBUtil
 package import SWBCore
+import Synchronization
 
 /// Information describing a complete build plan request.
 package struct BuildPlanRequest: Sendable {
@@ -187,6 +188,24 @@ package final class BuildPlan: StaleFileRemovalContext {
         }
 
         await aggregationQueue.sync{ }
+
+        // Create shared PCH ordering gate tasks. This must happen after all per target
+        // generateTasks() calls complete since those calls populate sharedPCHOrdering.
+        if !globalProductPlan.sharedPCHOrdering.isEmpty {
+            let workspaceResultContext = productPlanResultContexts.last!
+            globalProductPlan.sharedPCHOrdering.forEach { (_, ordering) in
+                let inputs = ordering.inputs.withLock { Array($0) }
+                guard !inputs.isEmpty else {
+                    return
+                }
+                let gateTask = delegate.createGateTask(inputs, output: ordering.orderingNode, name: ordering.orderingNode.name, mustPrecede: []) { _ in }
+                aggregationQueue.async {
+                    workspaceResultContext.addPlannedTasks([gateTask])
+                }
+            }
+            await aggregationQueue.sync{ }
+        }
+
         if delegate.cancelled {
             // Reset any deferred producers, which may participate in cycles.
             for context in productPlanResultContexts {

Sources/SWBTaskConstruction/ProductPlanning/ProductPlan.swift

@@ -47,6 +47,17 @@ package final class GlobalProductPlan: GlobalTargetInfoProvider
     /// This is a limited mechanism for concurrent task construction processes to coordinate on the production of a shared node.
     let sharedIntermediateNodes = Registry<String, (any PlannedNode, any Sendable)>()
 
+    /// Tracks ordering dependencies for shared PCHs.
+    final class SharedPCHOrdering: Sendable {
+        let orderingNode: any PlannedNode
+        let inputs: SWBMutex<[any PlannedNode]>
+        init(orderingNode: any PlannedNode, inputs: consuming SWBMutex<[any PlannedNode]>) {
+            self.orderingNode = orderingNode
+            self.inputs = inputs
+        }
+    }
+    let sharedPCHOrdering = Registry<String, SharedPCHOrdering>()
+
     let delegate: any GlobalProductPlanDelegate
 
     struct VFSContentsKey: Hashable {

Sources/SWBTaskConstruction/TaskProducers/BuildPhaseTaskProducers/SourcesTaskProducer.swift

@@ -180,6 +180,10 @@ private final class SourcesPhaseBasedTaskGenerationDelegate: TaskGenerationDeleg
         return delegate.createOrReuseSharedNodeWithIdentifier(ident, creator: creator)
     }
 
+    func registerSharedPCHOrderingDependency(_ ident: String, orderingNode: any PlannedNode) {
+        delegate.registerSharedPCHOrderingDependency(ident, orderingNode: orderingNode)
+    }
+
     func access(path: Path) {
         producer.access(path: path)
     }

Sources/SWBTaskConstruction/TaskProducers/OtherTaskProducers/ModuleMapTaskProducer.swift

@@ -127,6 +127,10 @@ private final class ModuleMapPhaseBasedTaskGenerationDelegate: TaskGenerationDel
         return delegate.createOrReuseSharedNodeWithIdentifier(ident, creator: creator)
     }
 
+    func registerSharedPCHOrderingDependency(_ ident: String, orderingNode: any PlannedNode) {
+        delegate.registerSharedPCHOrderingDependency(ident, orderingNode: orderingNode)
+    }
+
     func access(path: Path) {
         delegate.access(path: path)
     }

Sources/SWBTaskConstruction/TaskProducers/TaskProducer.swift

@@ -1713,6 +1713,17 @@ class ProducerBasedTaskGenerationDelegate: TaskGenerationDelegate {
         return context.globalProductPlan.sharedIntermediateNodes.getOrInsert(ident, creator)
     }
 
+    func registerSharedPCHOrderingDependency(_ ident: String, orderingNode: any PlannedNode) {
+        guard let configuredTarget = context.configuredTarget,
+              let targetGateNodes = context.globalProductPlan.targetGateNodes[configuredTarget] else {
+            return
+        }
+        let ordering = context.globalProductPlan.sharedPCHOrdering.getOrInsert(ident) {
+            GlobalProductPlan.SharedPCHOrdering(orderingNode: orderingNode, inputs: SWBMutex([]))
+        }
+        ordering.inputs.withLock { $0.append(targetGateNodes.startCompilingNode) }
+    }
+
     func access(path: Path) {
         producer.access(path: path)
     }

Sources/SWBTestSupport/CapturingTaskGenerationDelegate.swift

@@ -101,6 +101,7 @@ package class CapturingTaskGenerationDelegate: TaskGenerationDelegate, CoreClien
     package func createOrReuseSharedNodeWithIdentifier(_ ident: String, creator: () -> (any PlannedNode, any Sendable)) -> (any PlannedNode, any Sendable) {
         return sharedIntermediateNodes.getOrInsert(ident, creator)
     }
+    package func registerSharedPCHOrderingDependency(_ ident: String, orderingNode: any PlannedNode) {}
     package func access(path: Path) {}
     package func readFileContents(_ path: Path) throws -> ByteString { return ByteString() }
     package func fileExists(at path: Path) -> Bool { return true }

Tests/SWBCorePerfTests/CommandLineSpecPerfTests.swift

@@ -96,6 +96,7 @@ private final class CapturingTaskGenerationDelegate: TaskGenerationDelegate {
     {
         return sharedIntermediateNodes.getOrInsert(ident, creator)
     }
+    func registerSharedPCHOrderingDependency(_ ident: String, orderingNode: any PlannedNode) {}
     func access(path: Path) {}
     func readFileContents(_ path: Path) throws -> ByteString { return ByteString() }
     func recordAttachment(contents: SWBUtil.ByteString) -> SWBUtil.Path {

Tests/SWBCoreTests/CommandLineSpecTests.swift

@@ -959,7 +959,8 @@ import SWBMacro
         #expect(precompileTask.ruleInfo == ["ProcessPCH", Path.root.join("tmp/precomps/SharedPrecompiledHeaders/\(pchHash)/prefix.h.gch").str, Path.root.join("tmp/prefix.h").str, "normal", "x86_64", "c", "com.apple.compilers.llvm.clang.1_0"])
         #expect(precompileTask.execDescription == "Precompile prefix.h (x86_64)")
         precompileTask.checkInputs([
-            .path(Path.root.join("tmp/prefix.h").str)
+            .path(Path.root.join("tmp/prefix.h").str),
+            .any,
         ])
         precompileTask.checkOutputs([
             .path(Path.root.join("tmp/precomps/SharedPrecompiledHeaders/\(pchHash)/prefix.h.gch").str)