Fix and improve parallel mode

Parallel mode was crashing with bad memory access due to data races
accessing `Frontend.configurationLoader`, more specifically its `cache`.

After serializing access (initially with an `NSLock`), I observed that
despite not crashing anymore, the performance was surprisingly *worst*
than in single threaded mode.

That led me down a road of investigating why this was happening, and
after some profiling I discovered that `Rule`'s `nameCache` was the
main source of contention - causing around 30% of total spent time
waiting for locks (`ulock_wait`) on my tests.

After replacing the synchronization mechanism on `nameCache` with a
more efficient `os_unfair_lock_t` (`pthread_mutex_t` in Linux), the
contention dropped significantly, and parallel mode now outperformed
single threaded mode as expected. As a bonus, these changes also
improved single threaded mode performance as well, due to the reduced
overhead of using a lock vs a queue.

I then used the same `Lock` approach to serialize access to
`Frontend.configurationLoader` which increased the performance gap
even further.

After these improvements, I was able to obtain quite significant
performance gains using `Lock`:

- serial (non optimized) vs parallel (optimized): ~5.4x (13.5s vs 74s)
- serial (optimized) vs serial (non optimized): ~1.6x (44s vs 74s)
- serial (optimized) vs parallel (optimized): ~3.2x (13.5s vs 44s)

Sadly, a custom `Lock` implementation is not ideal for `swift-format`
to maintain and Windows support was not provided. As such, `NSLock` was
used instead which is a part of `Foundation` and supported on all major
platforms.

Using `NSLock` the improvements were not so good, unfortunately:

- serial (non optimized) vs parallel (NSLock): ~1,9x (38s vs 74s)
- serial (NSLock) vs serial (non optimized): ~1,4x (52s vs 74s)
- serial (NSLock) vs parallel (NSLock): ~1,3x (38s vs 52s)

Tests were made on my `MacBookPro16,1` (8-core [email protected]), on a project
with 2135 Swift files, compiling `swift-format` in Release mode.

## Changes

- Use an `NSLock` to serialize access to `Frontend.configurationLoader`.

- Use an `NSLock` to serialize access to `Rule`'s `nameCache`
(replacing `nameCacheQueue`)

Author: p4checo

Sources/SwiftFormatCore/Rule.swift

@@ -28,26 +28,22 @@ public protocol Rule {
 }
 
 fileprivate var nameCache = [ObjectIdentifier: String]()
-fileprivate var nameCacheQueue = DispatchQueue(
-      label: "com.apple.SwiftFormat.NameCache", attributes: .concurrent)
+fileprivate let nameCacheLock = NSLock()
 
 extension Rule {
   /// By default, the `ruleName` is just the name of the implementing rule class.
   public static var ruleName: String {
     let identifier = ObjectIdentifier(self)
-    let cachedName = nameCacheQueue.sync {
-        nameCache[identifier]
-    }
 
-    if let cachedName = cachedName {
+    nameCacheLock.lock()
+    defer { nameCacheLock.unlock() }
+
+    if let cachedName = nameCache[identifier] {
       return cachedName
     }
 
     let name = String("\(self)".split(separator: ".").last!)
-    nameCacheQueue.async(flags: .barrier) {
-        nameCache[identifier] = name
-    }
-
+    nameCache[identifier] = name
     return name
   }
 }

Sources/swift-format/Frontend/Frontend.swift

@@ -13,6 +13,7 @@
 import Foundation
 import SwiftFormat
 import SwiftFormatConfiguration
+import SwiftFormatCore
 import SwiftSyntax
 
 class Frontend {
@@ -78,6 +79,9 @@ class Frontend {
   /// Indicates whether any errors were emitted during execution.
   final var errorsWereEmitted: Bool { diagnosticEngine.hasErrors }
 
+  /// Lock to serialize access to shared state in parallel mode.
+  final let lock = NSLock()
+
   /// Creates a new frontend with the given options.
   ///
   /// - Parameter lintFormatOptions: Options that apply during formatting or linting.
@@ -181,7 +185,7 @@ class Frontend {
     // loaded. (Do not try to fall back to a path inferred from the source file path.)
     if let configurationFilePath = configurationFilePath {
       do {
-        return try configurationLoader.configuration(atPath: configurationFilePath)
+        return try lock.sync { try configurationLoader.configuration(atPath: configurationFilePath) }
       } catch {
         diagnosticEngine.diagnose(
           Diagnostic.Message(.error, "Unable to read configuration: \(error.localizedDescription)"))
@@ -194,7 +198,8 @@ class Frontend {
     if let swiftFilePath = swiftFilePath {
       do {
         if let configuration =
-          try configurationLoader.configuration(forSwiftFileAtPath: swiftFilePath)
+            try lock.sync(
+              work: { try configurationLoader.configuration(forSwiftFileAtPath: swiftFilePath) })
         {
           return configuration
         }
@@ -213,3 +218,13 @@ class Frontend {
     return Configuration()
   }
 }
+
+private extension NSLock {
+
+  @discardableResult
+  func sync<R>(work: () throws -> R) rethrows -> R {
+    lock()
+    defer { unlock() }
+    return try work()
+  }
+}