From e38b96e41a6dfb8e1e4ce0a2c1fa1a9136cd456a Mon Sep 17 00:00:00 2001 From: Boris Buegling Date: Thu, 13 Nov 2025 13:06:00 -0800 Subject: [PATCH] Further reduce lock contention around `macroConfigPaths` We were still going through the lock once for every `MacroValueAssignment` which was causing performance issues during config file parsing. This didn't really make sense since we can instead do the insert in `MacroConfigFileParser` and otherwise only deal in index references. Real locations only need to be materialized for code that deals with them. rdar://164650188 --- Sources/SWBCore/MacroConfigFileLoader.swift | 10 ++--- Sources/SWBMacro/MacroConfigFileParser.swift | 9 ++-- Sources/SWBMacro/MacroEvaluationScope.swift | 2 +- .../SWBMacro/MacroValueAssignmentTable.swift | 45 ++++--------------- Tests/SWBCoreTests/SettingsTests.swift | 2 +- Tests/SWBMacroTests/MacroParsingTests.swift | 4 +- 6 files changed, 23 insertions(+), 49 deletions(-) diff --git a/Sources/SWBCore/MacroConfigFileLoader.swift b/Sources/SWBCore/MacroConfigFileLoader.swift index d9153351..705f9940 100644 --- a/Sources/SWBCore/MacroConfigFileLoader.swift +++ b/Sources/SWBCore/MacroConfigFileLoader.swift @@ -248,7 +248,7 @@ final class MacroConfigFileLoader: Sendable { return MacroConfigFileParser(byteString: data, path: path, delegate: delegate) } - mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) { + mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, pathRef: OrderedSet.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) { // Look up the macro name, creating it as a user-defined macro if it isn’t already known. let macro = table.namespace.lookupOrDeclareMacro(UserDefinedMacroDeclaration.self, macroName) @@ -259,8 +259,8 @@ final class MacroConfigFileLoader: Sendable { } // Parse the value in a manner consistent with the macro definition. - let location = MacroValueAssignmentLocation(path: path, startLine: startLine, endLine: endLine, startColumn: startColumn, endColumn: endColumn) - table.push(macro, table.namespace.parseForMacro(macro, value: value), conditions: conditionSet, location: location) + let locationRef = InternedMacroValueAssignmentLocation(pathRef: pathRef, startLine: startLine, endLine: endLine, startColumn: startColumn, endColumn: endColumn) + table.push(macro, table.namespace.parseForMacro(macro, value: value), conditions: conditionSet, locationRef: locationRef) } func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser) { @@ -308,8 +308,8 @@ fileprivate final class MacroValueAssignmentTableRef { table.namespace } - func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil, location: MacroValueAssignmentLocation? = nil) { - table.push(macro, value, conditions: conditions, location: location) + func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil, locationRef: InternedMacroValueAssignmentLocation? = nil) { + table.push(macro, value, conditions: conditions, locationRef: locationRef) } } diff --git a/Sources/SWBMacro/MacroConfigFileParser.swift b/Sources/SWBMacro/MacroConfigFileParser.swift index 4e5ddc76..63610a08 100644 --- a/Sources/SWBMacro/MacroConfigFileParser.swift +++ b/Sources/SWBMacro/MacroConfigFileParser.swift @@ -12,6 +12,7 @@ public import SWBUtil import Foundation +import Synchronization private extension UInt8 { var isASCIISpace: Bool { @@ -35,6 +36,7 @@ public final class MacroConfigFileParser { /// The path to the xcconfig file being parsed, for diagnostic purposes. public let path: Path + private let pathRef: OrderedSet.Index /// Current index into the UTF-8 byte sequence that’s being parsed. Starts at zero. All meaningful characters are in the ASCII range, which is what lets us do this optimization. var currIdx: Int @@ -57,6 +59,7 @@ public final class MacroConfigFileParser { self.delegate = delegate self.bytes = byteString.bytes self.path = path + self.pathRef = MacroValueAssignment.macroConfigPaths.withLock({ $0.append(path).index }) self.currIdx = 0 self.currLine = 1 } @@ -399,7 +402,7 @@ public final class MacroConfigFileParser { } // Finally, now that we have the name, conditions, and value, we tell the delegate about it. let value = chunks.joined(separator: " ") - delegate?.foundMacroValueAssignment(name, conditions: conditions, value: value, path: path, startLine: startLine, endLine: currLine, startColumn: startColumn, endColumn: currIdx - startOfLine, parser: self) + delegate?.foundMacroValueAssignment(name, conditions: conditions, value: value, path: path, pathRef: pathRef, startLine: startLine, endLine: currLine, startColumn: startColumn, endColumn: currIdx - startOfLine, parser: self) } public func parseNonListAssignmentRHS() -> String? { @@ -538,7 +541,7 @@ public final class MacroConfigFileParser { } func endPreprocessorInclusion() { } - func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) { + func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, pathRef: OrderedSet.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) { self.macroName = macroName self.conditions = conditions.isEmpty ? nil : conditions } @@ -585,7 +588,7 @@ public protocol MacroConfigFileParserDelegate { func endPreprocessorInclusion() /// Invoked once for each macro value assignment. The `macroName` is guaranteed to be non-empty, but `value` may be empty. Any macro conditions are passed as tuples in the `conditions`; parameters are guaranteed to be non-empty strings, but patterns may be empty. - mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) + mutating func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, pathRef: OrderedSet.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) /// Invoked if an error, warning, or other diagnostic is detected. func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser) diff --git a/Sources/SWBMacro/MacroEvaluationScope.swift b/Sources/SWBMacro/MacroEvaluationScope.swift index f8c739e2..0c4c5e14 100644 --- a/Sources/SWBMacro/MacroEvaluationScope.swift +++ b/Sources/SWBMacro/MacroEvaluationScope.swift @@ -17,7 +17,7 @@ private extension MacroValueAssignmentTable { func lookupMacro(_ macro: MacroDeclaration, overrideLookup: ((MacroDeclaration) -> MacroExpression?)? = nil) -> MacroValueAssignment? { // See if we have an overriding binding. if let override = overrideLookup?(macro) { - return MacroValueAssignment(expression: override, conditions: nil, next: lookupMacro(macro), location: nil) + return MacroValueAssignment(expression: override, conditions: nil, next: lookupMacro(macro), locationRef: nil) } // Otherwise, return the normal lookup. diff --git a/Sources/SWBMacro/MacroValueAssignmentTable.swift b/Sources/SWBMacro/MacroValueAssignmentTable.swift index 788dbd68..6357b827 100644 --- a/Sources/SWBMacro/MacroValueAssignmentTable.swift +++ b/Sources/SWBMacro/MacroValueAssignmentTable.swift @@ -11,7 +11,7 @@ //===----------------------------------------------------------------------===// public import SWBUtil -import Synchronization +public import Synchronization /// A mapping from macro declarations to corresponding macro value assignments, each of which is a linked list of macro expressions in precedence order. At the moment it doesn’t support conditional assignments, but that functionality will be implemented soon. public struct MacroValueAssignmentTable: Serializable, Sendable { @@ -78,14 +78,7 @@ public struct MacroValueAssignmentTable: Serializable, Sendable { /// Adds a mapping from `macro` to `value`, inserting it ahead of any already existing assignment for the same macro. Unless the value refers to the lower-precedence expression (using `$(inherited)` notation), any existing assignments are shadowed but not removed. - public mutating func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil, location: MacroValueAssignmentLocation? = nil) { - assert(namespace.lookupMacroDeclaration(macro.name) === macro) - // Validate the type. - assert(macro.type.matchesExpressionType(value)) - valueAssignments[macro] = MacroValueAssignment(expression: value, conditions: conditions, next: valueAssignments[macro], location: location) - } - - mutating func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil, locationRef: InternedMacroValueAssignmentLocation?) { + package mutating func push(_ macro: MacroDeclaration, _ value: MacroExpression, conditions: MacroConditionSet? = nil, locationRef: InternedMacroValueAssignmentLocation? = nil) { assert(namespace.lookupMacroDeclaration(macro.name) === macro) // Validate the type. assert(macro.type.matchesExpressionType(value)) @@ -338,7 +331,7 @@ public final class MacroValueAssignment: Serializable, CustomStringConvertible, public let next: MacroValueAssignment? let _location: InternedMacroValueAssignmentLocation? - private static let macroConfigPaths = SWBMutex>(OrderedSet()) + package static let macroConfigPaths = SWBMutex>(OrderedSet()) public var location: MacroValueAssignmentLocation? { if let _location { @@ -355,28 +348,6 @@ public final class MacroValueAssignment: Serializable, CustomStringConvertible, } /// Initializes the macro value assignment to represent `expression`, with the next existing macro value assignment (if any). - convenience init(expression: MacroExpression, conditions: MacroConditionSet? = nil, next: MacroValueAssignment?, location: MacroValueAssignmentLocation?) { - let locationRef: InternedMacroValueAssignmentLocation? - if let location { - locationRef = InternedMacroValueAssignmentLocation( - pathRef: Self.macroConfigPaths.withLock({ $0.append(location.path).index }), - startLine: location.startLine, - endLine: location.endLine, - startColumn: location.startColumn, - endColumn: location.endColumn - ) - } else { - locationRef = nil - } - - self.init( - expression: expression, - conditions: conditions, - next: next, - locationRef: locationRef - ) - } - init(expression: MacroExpression, conditions: MacroConditionSet? = nil, next: MacroValueAssignment?, locationRef: InternedMacroValueAssignmentLocation?) { self.expression = expression self.conditions = conditions @@ -457,7 +428,7 @@ public struct MacroValueAssignmentLocation: Sendable, Equatable { public let startColumn: Int public let endColumn: Int - public init(path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int) { + @_spi(Testing) public init(path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int) { self.path = path self.startLine = startLine self.endLine = endLine @@ -466,14 +437,14 @@ public struct MacroValueAssignmentLocation: Sendable, Equatable { } } -struct InternedMacroValueAssignmentLocation: Serializable, Sendable { +package struct InternedMacroValueAssignmentLocation: Serializable, Sendable { let pathRef: OrderedSet.Index public let startLine: Int public let endLine: Int let startColumn: Int let endColumn: Int - init(pathRef: OrderedSet.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int) { + package init(pathRef: OrderedSet.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int) { self.pathRef = pathRef self.startLine = startLine self.endLine = endLine @@ -516,10 +487,10 @@ private func insertCopiesOfMacroValueAssignmentNodes(_ srcAsgn: MacroValueAssign } if let srcNext = srcAsgn.next { - return MacroValueAssignment(expression: srcAsgn.expression, conditions:srcAsgn.conditions, next: insertCopiesOfMacroValueAssignmentNodes(srcNext, inFrontOf: dstAsgn), location: srcAsgn.location) + return MacroValueAssignment(expression: srcAsgn.expression, conditions:srcAsgn.conditions, next: insertCopiesOfMacroValueAssignmentNodes(srcNext, inFrontOf: dstAsgn), locationRef: srcAsgn._location) } else { - return MacroValueAssignment(expression: srcAsgn.expression, conditions:srcAsgn.conditions, next: dstAsgn, location: srcAsgn.location) + return MacroValueAssignment(expression: srcAsgn.expression, conditions:srcAsgn.conditions, next: dstAsgn, locationRef: srcAsgn._location) } } diff --git a/Tests/SWBCoreTests/SettingsTests.swift b/Tests/SWBCoreTests/SettingsTests.swift index 333db96d..515f71a2 100644 --- a/Tests/SWBCoreTests/SettingsTests.swift +++ b/Tests/SWBCoreTests/SettingsTests.swift @@ -17,7 +17,7 @@ import Testing import SWBProtocol import SWBTestSupport @_spi(Testing) import SWBUtil -import SWBMacro +@_spi(Testing) import SWBMacro @_spi(Testing) import SWBCore @Suite fileprivate struct SettingsTests: CoreBasedTests { diff --git a/Tests/SWBMacroTests/MacroParsingTests.swift b/Tests/SWBMacroTests/MacroParsingTests.swift index 84593196..b9b408a8 100644 --- a/Tests/SWBMacroTests/MacroParsingTests.swift +++ b/Tests/SWBMacroTests/MacroParsingTests.swift @@ -790,7 +790,7 @@ fileprivate let testFileData = [ } func endPreprocessorInclusion() { } - func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) { + func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, pathRef: OrderedSet.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) { } func handleDiagnostic(_ diagnostic: MacroConfigFileDiagnostic, parser: MacroConfigFileParser) { @@ -895,7 +895,7 @@ private func TestMacroConfigFileParser(_ string: String, expectedAssignments: [A func endPreprocessorInclusion() { self.includeDirectivesCount += 1 } - func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) { + func foundMacroValueAssignment(_ macroName: String, conditions: [(param: String, pattern: String)], value: String, path: Path, pathRef: OrderedSet.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int, parser: MacroConfigFileParser) { // print("\(parser.lineNumber): \(macroName)\(conditions.map({ "[\($0.param)=\($0.pattern)]" }).joinWithSeparator(""))=\(value)") assignments.append((macro: macroName, conditions: conditions, value: value)) locations.append((macro: macroName, path: path, startLine: startLine, endLine: endLine, startColumn: startColumn, endColumn: endColumn))