Merge pull request #542 from swiftlang/owenv/single-use-backtraces

Improve task backtraces for dynamic tasks

Author: owenv

Package.swift

@@ -369,7 +369,7 @@ let package = Package(
         // Perf tests
         .testTarget(
             name: "SWBBuildSystemPerfTests",
-            dependencies: ["SWBBuildSystem", "SWBTestSupport"],
+            dependencies: ["SWBBuildSystem", "SWBTestSupport", "SwiftBuildTestSupport"],
             swiftSettings: swiftSettings(languageMode: .v6)),
         .testTarget(
             name: "SWBCASPerfTests",

Sources/SWBProtocol/BuildOperationMessages.swift

@@ -126,7 +126,7 @@ public struct BuildOperationTargetInfo: SerializableCodable, Equatable, Sendable
     }
 }
 
-public enum BuildOperationTaskSignature: RawRepresentable, Sendable, Hashable, Codable, CustomDebugStringConvertible {
+public enum BuildOperationTaskSignature: RawRepresentable, Sendable, Comparable, Hashable, Codable, CustomDebugStringConvertible {
     case taskIdentifier(ByteString)
     case activitySignature(ByteString)
     case subtaskSignature(ByteString)
@@ -155,6 +155,10 @@ public enum BuildOperationTaskSignature: RawRepresentable, Sendable, Hashable, C
         }
     }
 
+    public static func < (lhs: BuildOperationTaskSignature, rhs: BuildOperationTaskSignature) -> Bool {
+        lhs.rawValue.lexicographicallyPrecedes(rhs.rawValue)
+    }
+
     public init(from decoder: any Decoder) throws {
         let container = try decoder.singleValueContainer()
         guard let value = BuildOperationTaskSignature(rawValue: ByteString(try container.decode([UInt8].self))) else {
@@ -1020,7 +1024,7 @@ public struct BuildOperationDiagnosticEmitted: Message, Equatable, SerializableC
     }
 }
 
-public struct BuildOperationBacktraceFrameEmitted: Message, Equatable, SerializableCodable {
+public struct BuildOperationBacktraceFrameEmitted: Message, Equatable, Hashable, SerializableCodable {
     public static let name = "BUILD_BACKTRACE_FRAME_EMITTED"
 
     public enum Identifier: Hashable, Equatable, Comparable, SerializableCodable, Sendable {

Sources/SWBTestSupport/BuildOperationTester.swift

@@ -155,7 +155,7 @@ package final class BuildOperationTester {
         case subtaskDidReportProgress(SubtaskProgressEvent, count: Int)
 
         /// The build emitted a backtrace frame.
-        case emittedBuildBacktraceFrame(identifier: SWBProtocol.BuildOperationBacktraceFrameEmitted.Identifier, previousFrameIdentifier: SWBProtocol.BuildOperationBacktraceFrameEmitted.Identifier?, category: SWBProtocol.BuildOperationBacktraceFrameEmitted.Category, description: String)
+        case emittedBuildBacktraceFrame(BuildOperationBacktraceFrameEmitted)
 
         package var description: String {
             switch self {
@@ -189,8 +189,8 @@ package final class BuildOperationTester {
                 return "activityEmittedData(\(ruleInfo), bytes: \(ByteString(bytes).asString)"
             case .activityEnded(ruleInfo: let ruleInfo):
                 return "activityEnded(\(ruleInfo))"
-            case .emittedBuildBacktraceFrame(identifier: let id, previousFrameIdentifier: let previousID, category: let category, description: let description):
-                return "emittedBuildBacktraceFrame(\(id), previous: \(String(describing: previousID)), category: \(category), description: \(description))"
+            case .emittedBuildBacktraceFrame(let frame):
+                return "emittedBuildBacktraceFrame(\(frame.identifier), previous: \(String(describing: frame.previousFrameIdentifier)), category: \(frame.category), description: \(frame.description))"
             case .previouslyBatchedSubtaskUpToDate(let signature):
                 return "previouslyBatchedSubtaskUpToDate(\(signature))"
             }
@@ -735,18 +735,6 @@ package final class BuildOperationTester {
 
         }
 
-        package func checkNoTaskWithBacktraces(_ conditions: TaskCondition..., sourceLocation: SourceLocation = #_sourceLocation) {
-            for matchedTask in findMatchingTasks(conditions) {
-                Issue.record("found unexpected task matching conditions '\(conditions)', found: \(matchedTask)", sourceLocation: sourceLocation)
-
-                if let frameID = getBacktraceID(matchedTask, sourceLocation: sourceLocation) {
-                    enumerateBacktraces(frameID) { _, category, description in
-                        Issue.record("...<category='\(category)' description='\(description)'>", sourceLocation: sourceLocation)
-                    }
-                }
-            }
-        }
-
         /// Check whether the results contains a dependency cycle error. If so, then consume the error and create a `CycleChecking` object and pass it to the block. Otherwise fail.
         package func checkDependencyCycle(_ pattern: StringPattern, kind: DiagnosticKind = .error, failIfNotFound: Bool = true, sourceLocation: SourceLocation = #_sourceLocation, body: (CycleChecker) async throws -> Void) async throws {
             guard let message = getDiagnosticMessage(pattern, kind: kind, checkDiagnostic: { _ in true }) else {
@@ -1045,55 +1033,6 @@ package final class BuildOperationTester {
             startedTasks.remove(task)
         }
 
-        private func getBacktraceID(_ task: Task, sourceLocation: SourceLocation = #_sourceLocation) -> BuildOperationBacktraceFrameEmitted.Identifier? {
-            guard let frameID: BuildOperationBacktraceFrameEmitted.Identifier = events.compactMap ({ (event) -> BuildOperationBacktraceFrameEmitted.Identifier? in
-                guard case .emittedBuildBacktraceFrame(identifier: let identifier, previousFrameIdentifier: _, category: _, description: _) = event, case .task(let signature) = identifier, BuildOperationTaskSignature.taskIdentifier(ByteString(encodingAsUTF8: task.identifier.rawValue)) == signature else {
-                    return nil
-                }
-                return identifier
-                // Iff the task is a dynamic task, there may be more than one corresponding frame if it was requested multiple times, in which case we choose the first. Non-dynamic tasks always have a 1-1 relationship with frames.
-            }).sorted().first else {
-                Issue.record("Did not find a single build backtrace frame for task: \(task.identifier)", sourceLocation: sourceLocation)
-                return nil
-            }
-            return frameID
-        }
-
-        private func enumerateBacktraces(_ identifier: BuildOperationBacktraceFrameEmitted.Identifier, _ handleFrameInfo: (_ identifier: BuildOperationBacktraceFrameEmitted.Identifier?, _ category: BuildOperationBacktraceFrameEmitted.Category, _ description: String) -> ()) {
-            var currentFrameID: BuildOperationBacktraceFrameEmitted.Identifier? = identifier
-            while let id = currentFrameID {
-                if let frameInfo: (BuildOperationBacktraceFrameEmitted.Identifier?, BuildOperationBacktraceFrameEmitted.Category, String) = events.compactMap({ (event) -> (BuildOperationBacktraceFrameEmitted.Identifier?, BuildOperationBacktraceFrameEmitted.Category, String)? in
-                    guard case .emittedBuildBacktraceFrame(identifier: id, previousFrameIdentifier: let previousFrameIdentifier, category: let category, description: let description) = event else {
-                        return nil
-                    }
-                    return (previousFrameIdentifier, category, description)
-                    // Iff the task is a dynamic task, there may be more than one corresponding frame if it was requested multiple times, in which case we choose the first. Non-dynamic tasks always have a 1-1 relationship with frames.
-                }).sorted(by: { $0.0 }).first {
-                    handleFrameInfo(frameInfo.0, frameInfo.1, frameInfo.2)
-                    currentFrameID = frameInfo.0
-                } else {
-                    currentFrameID = nil
-                }
-            }
-        }
-
-        package func checkBacktrace(_ identifier: BuildOperationBacktraceFrameEmitted.Identifier, _ patterns: [StringPattern], sourceLocation: SourceLocation = #_sourceLocation) {
-            var frameDescriptions: [String] = []
-            enumerateBacktraces(identifier) { (_, category, description) in
-                frameDescriptions.append("<category='\(category)' description='\(description)'>")
-            }
-
-            XCTAssertMatch(frameDescriptions, patterns, sourceLocation: sourceLocation)
-        }
-
-        package func checkBacktrace(_ task: Task, _ patterns: [StringPattern], sourceLocation: SourceLocation = #_sourceLocation) {
-            if let frameID = getBacktraceID(task, sourceLocation: sourceLocation) {
-                checkBacktrace(frameID, patterns, sourceLocation: sourceLocation)
-            } else {
-                // already recorded an issue
-            }
-        }
-
         private class TaskDependencyResolver {
             /// The database schema has to match what `BuildSystemImpl` defines in `getMergedSchemaVersion()`.
             /// Can be removed once rdar://85336712 is resolved.
@@ -1563,42 +1502,6 @@ package final class BuildOperationTester {
         }
     }
 
-    /// Ensure that the build is a null build.
-    package func checkNullBuild(_ name: String? = nil, parameters: BuildParameters? = nil, runDestination: RunDestinationInfo?, buildRequest inputBuildRequest: BuildRequest? = nil, buildCommand: BuildCommand? = nil, schemeCommand: SchemeCommand? = .launch, persistent: Bool = false, serial: Bool = false, buildOutputMap: [String:String]? = nil, signableTargets: Set<String> = [], signableTargetInputs: [String: ProvisioningTaskInputs] = [:], clientDelegate: (any ClientDelegate)? = nil, excludedTasks: Set<String> = ["ClangStatCache", "LinkAssetCatalogSignature"], diagnosticsToValidate: Set<DiagnosticKind> = [.note, .error, .warning], sourceLocation: SourceLocation = #_sourceLocation) async throws {
-
-        func body(results: BuildResults) throws -> Void {
-            results.consumeTasksMatchingRuleTypes(excludedTasks)
-            results.checkNoTaskWithBacktraces(sourceLocation: sourceLocation)
-
-            results.checkNote(.equal("Building targets in dependency order"), failIfNotFound: false)
-            results.checkNote(.prefix("Target dependency graph"), failIfNotFound: false)
-
-            for kind in diagnosticsToValidate {
-                switch kind {
-                case .note:
-                    results.checkNoNotes(sourceLocation: sourceLocation)
-
-                case .warning:
-                    results.checkNoWarnings(sourceLocation: sourceLocation)
-
-                case .error:
-                    results.checkNoErrors(sourceLocation: sourceLocation)
-
-                case .remark:
-                    results.checkNoRemarks(sourceLocation: sourceLocation)
-
-                default:
-                    // other kinds are ignored
-                    break
-                }
-            }
-        }
-
-        try await UserDefaults.withEnvironment(["EnableBuildBacktraceRecording": "true"]) {
-            try await checkBuild(name, parameters: parameters, runDestination: runDestination, buildRequest: inputBuildRequest, buildCommand: buildCommand, schemeCommand: schemeCommand, persistent: persistent, serial: serial, buildOutputMap: buildOutputMap, signableTargets: signableTargets, signableTargetInputs: signableTargetInputs, clientDelegate: clientDelegate, sourceLocation: sourceLocation, body: body)
-        }
-    }
-
     package static func buildRequestForIndexOperation(
         workspace: Workspace,
         buildTargets: [any TestTarget]? = nil,
@@ -2252,7 +2155,7 @@ private final class BuildOperationTesterDelegate: BuildOperationDelegate {
 
     func recordBuildBacktraceFrame(identifier: SWBProtocol.BuildOperationBacktraceFrameEmitted.Identifier, previousFrameIdentifier: SWBProtocol.BuildOperationBacktraceFrameEmitted.Identifier?, category: SWBProtocol.BuildOperationBacktraceFrameEmitted.Category, kind: SWBProtocol.BuildOperationBacktraceFrameEmitted.Kind, description: String) {
         queue.async {
-            self.events.append(.emittedBuildBacktraceFrame(identifier: identifier, previousFrameIdentifier: previousFrameIdentifier, category: category, description: description))
+            self.events.append(.emittedBuildBacktraceFrame(.init(identifier: identifier, previousFrameIdentifier: previousFrameIdentifier, category: category, kind: kind, description: description)))
         }
     }
 }

Sources/SwiftBuild/SWBBuildOperationBacktraceFrame.swift

@@ -15,9 +15,9 @@ import SWBUtil
 
 public import Foundation
 
-public struct SWBBuildOperationBacktraceFrame: Hashable, Sendable, Codable, Identifiable {
-    public struct Identifier: Equatable, Hashable, Sendable, Codable, CustomDebugStringConvertible {
-        private enum Storage: Equatable, Hashable, Sendable, Codable {
+public struct SWBBuildOperationBacktraceFrame: Hashable, Sendable, Codable, Identifiable, Comparable {
+    public struct Identifier: Equatable, Comparable, Hashable, Sendable, Codable, CustomDebugStringConvertible {
+        private enum Storage: Equatable, Comparable, Hashable, Sendable, Codable {
             case task(BuildOperationTaskSignature)
             case key(String)
         }
@@ -39,6 +39,10 @@ public struct SWBBuildOperationBacktraceFrame: Hashable, Sendable, Codable, Iden
             self.storage = .task(taskSignature)
         }
 
+        package init(genericBuildKey: String) {
+            self.storage = .key(genericBuildKey)
+        }
+
         public var debugDescription: String {
             switch storage {
             case .task(let taskSignature):
@@ -47,9 +51,13 @@ public struct SWBBuildOperationBacktraceFrame: Hashable, Sendable, Codable, Iden
                 return key
             }
         }
+
+        public static func < (lhs: SWBBuildOperationBacktraceFrame.Identifier, rhs: SWBBuildOperationBacktraceFrame.Identifier) -> Bool {
+            lhs.storage < rhs.storage
+        }
     }
 
-    public enum Category: Equatable, Hashable, Sendable, Codable {
+    public enum Category: Equatable, Comparable, Hashable, Sendable, Codable {
         case ruleNeverBuilt
         case ruleSignatureChanged
         case ruleHadInvalidValue
@@ -68,7 +76,7 @@ public struct SWBBuildOperationBacktraceFrame: Hashable, Sendable, Codable, Iden
             }
         }
     }
-    public enum Kind: Equatable, Hashable, Sendable, Codable {
+    public enum Kind: Equatable, Comparable, Hashable, Sendable, Codable {
         case genericTask
         case swiftDriverJob
         case file
@@ -82,6 +90,14 @@ public struct SWBBuildOperationBacktraceFrame: Hashable, Sendable, Codable, Iden
     public let description: String
     public let frameKind: Kind
 
+    package init(identifier: Identifier, previousFrameIdentifier: Identifier?, category: Category, description: String, frameKind: Kind) {
+        self.identifier = identifier
+        self.previousFrameIdentifier = previousFrameIdentifier
+        self.category = category
+        self.description = description
+        self.frameKind = frameKind
+    }
+
     // The old name collides with the `kind` key used in the SwiftBuildMessage JSON encoding
     @available(*, deprecated, renamed: "frameKind")
     public var kind: Kind {
@@ -91,6 +107,10 @@ public struct SWBBuildOperationBacktraceFrame: Hashable, Sendable, Codable, Iden
     public var id: Identifier {
         identifier
     }
+
+    public static func < (lhs: SWBBuildOperationBacktraceFrame, rhs: SWBBuildOperationBacktraceFrame) -> Bool {
+        (lhs.identifier, lhs.previousFrameIdentifier, lhs.category, lhs.description, lhs.frameKind) < (rhs.identifier, rhs.previousFrameIdentifier, rhs.category, rhs.description, rhs.frameKind)
+    }
 }
 
 extension SWBBuildOperationBacktraceFrame {
@@ -134,3 +154,43 @@ extension SWBBuildOperationBacktraceFrame {
         self.init(identifier: id, previousFrameIdentifier: previousID, category: category, description: message.description, frameKind: kind)
     }
 }
+
+public struct SWBBuildOperationCollectedBacktraceFrames {
+    fileprivate var frames: [SWBBuildOperationBacktraceFrame.Identifier: Set<SWBBuildOperationBacktraceFrame>]
+
+    public init() {
+        self.frames = [:]
+    }
+
+    public mutating func add(frame: SWBBuildOperationBacktraceFrame) {
+        frames[frame.identifier, default: []].insert(frame)
+    }
+}
+
+public struct SWBTaskBacktrace {
+    public let frames: [SWBBuildOperationBacktraceFrame]
+
+    public init?(from baseFrameID: SWBBuildOperationBacktraceFrame.Identifier, collectedFrames: SWBBuildOperationCollectedBacktraceFrames) {
+        var frames: [SWBBuildOperationBacktraceFrame] = []
+        var currentFrame = collectedFrames.frames[baseFrameID]?.only
+        while let frame = currentFrame {
+            frames.append(frame)
+            if let previousFrameID = frame.previousFrameIdentifier, let candidatesForNextFrame = collectedFrames.frames[previousFrameID] {
+                switch frame.category {
+                case .dynamicTaskRegistration:
+                    currentFrame = candidatesForNextFrame.sorted().first {
+                        $0.category == .dynamicTaskRequest
+                    }
+                default:
+                    currentFrame = candidatesForNextFrame.sorted().first
+                }
+            } else {
+                currentFrame = nil
+            }
+        }
+        guard !frames.isEmpty else {
+            return nil
+        }
+        self.frames = frames
+    }
+}