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

Author: neonichu

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<Path>.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)
     }
 }
 

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<Path>.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<Path>.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<Path>.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)

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.

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<Path>>(OrderedSet())
+    package static let macroConfigPaths = SWBMutex<OrderedSet<Path>>(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<Path>.Index
     public let startLine: Int
     public let endLine: Int
     let startColumn: Int
     let endColumn: Int
 
-    init(pathRef: OrderedSet<Path>.Index, startLine: Int, endLine: Int, startColumn: Int, endColumn: Int) {
+    package init(pathRef: OrderedSet<Path>.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)
     }
 }
 

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 {

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<Path>.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<Path>.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))