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, The
alternative would be to use `NSLock` which being a part of `Foundation`
is 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)

The best solution was then to try and remove all concurrency contention
from `swift-format`. By flattening the `FileToProcess` array and
making the `Rule`'s `nameCache` static, close to optimal performance
should be achievable.

Base rules `SyntaxFormatRule` and `SyntaxLintRule` should be kept
in the `SwiftFormatCore` target to eventually support dynamic
discovery of rules (ditching `generate-pipeline`) and only require
rule implementers to import `SwiftFormatCore`.

On the other hand, any statically generated `ruleName` cache must be in
`SwiftFormatRules` target, because  that's where rule implementations
reside to capture their `ObjectIdentifier` at compile time.

So, in order to have a static rule name cache that's accessible from
`SwiftFormatCore` (especially from `SyntaxFormatRule`), a solution was
to inject a `ruleNameCache` dictionary in `Context`, which is injected
into every `Rule`.

This allows using `generate-pipelines` to a produce a `ruleNameCache`
in `SwiftFormatRules` that's injected on all `Context`s by each of the
pipelines (which is done in `SwiftFormat` target).

While not as ideal as having the cached accessed in a single place
(this approach incurs in the cache being copied for each `Context`), it
achieves a good tradeoff given current constraints). Lets hope the
compiler is smart enough to optimize this somehow.

All things considered, results were quite good:

- serial (non optimized) vs parallel (optimized): ~7x (10.5s vs 74s)
- serial (optimized) vs serial (non optimized): ~1.7x (42.5s vs 74s)
- serial (optimized) vs parallel (optimized): ~4x (10.5s vs 42.5s)

Tests were made on my `MacBookPro16,1` (8-core i9@2.4GHz), on a project
with 2135 Swift files, compiling `swift-format` in Release mode.

## Changes

- Make file preparation serial by calculating all `FileToProcess`
before processing them in parallel.

- Update `Context` to receive a `ruleNameCache` of type
`[ObjectIdentifier: String]` on `init`.

- Update `Context.isRuleEnabled` to receive the `Rule` type instead of
the rule's name, to use the new cache.

- Add new `RuleNameCacheGenerator` to `generate-pipeline` which
outputs a static `ruleNameCache` dictionary to
`RuleNameCache+Generated.swift` file in `SwiftFormatRule` target with
all existing rules.

- Remove the `nameCache` and `nameCacheQueue` `Rule.swift`.

Package.swift

@@ -47,7 +47,11 @@ let package = Package(
     ),
     .target(
       name: "SwiftFormatTestSupport",
-      dependencies: ["SwiftFormatCore", "SwiftFormatConfiguration"]
+      dependencies: [
+        "SwiftFormatCore",
+        "SwiftFormatRules",
+        "SwiftFormatConfiguration",
+      ]
     ),
     .target(
       name: "SwiftFormatWhitespaceLinter",

Sources/SwiftFormat/LintPipeline.swift

@@ -29,7 +29,7 @@ extension LintPipeline {
   func visitIfEnabled<Rule: SyntaxLintRule, Node: SyntaxProtocol>(
     _ visitor: (Rule) -> (Node) -> SyntaxVisitorContinueKind, for node: Node
   ) {
-    guard context.isRuleEnabled(Rule.self.ruleName, node: Syntax(node)) else { return }
+    guard context.isRuleEnabled(Rule.self, node: Syntax(node)) else { return }
     let rule = self.rule(Rule.self)
     _ = visitor(rule)(node)
   }
@@ -50,7 +50,7 @@ extension LintPipeline {
     // more importantly because the `visit` methods return protocol refinements of `Syntax` that
     // cannot currently be expressed as constraints without duplicating this function for each of
     // them individually.
-    guard context.isRuleEnabled(Rule.self.ruleName, node: Syntax(node)) else { return }
+    guard context.isRuleEnabled(Rule.self, node: Syntax(node)) else { return }
     let rule = self.rule(Rule.self)
     _ = visitor(rule)(node)
   }

Sources/SwiftFormat/SwiftFormatter.swift

@@ -14,6 +14,7 @@ import Foundation
 import SwiftFormatConfiguration
 import SwiftFormatCore
 import SwiftFormatPrettyPrint
+import SwiftFormatRules
 import SwiftSyntax
 
 /// Formats Swift source code or syntax trees according to the Swift style guidelines.
@@ -108,7 +109,7 @@ public final class SwiftFormatter {
     let assumedURL = url ?? URL(fileURLWithPath: "source")
     let context = Context(
       configuration: configuration, diagnosticEngine: diagnosticEngine, fileURL: assumedURL,
-      sourceFileSyntax: syntax, source: source)
+      sourceFileSyntax: syntax, source: source, ruleNameCache: ruleNameCache)
     let pipeline = FormatPipeline(context: context)
     let transformedSyntax = pipeline.visit(Syntax(syntax))
 

Sources/SwiftFormat/SwiftLinter.swift

@@ -14,6 +14,7 @@ import Foundation
 import SwiftFormatConfiguration
 import SwiftFormatCore
 import SwiftFormatPrettyPrint
+import SwiftFormatRules
 import SwiftFormatWhitespaceLinter
 import SwiftSyntax
 
@@ -88,7 +89,7 @@ public final class SwiftLinter {
 
     let context = Context(
       configuration: configuration, diagnosticEngine: diagnosticEngine, fileURL: url,
-      sourceFileSyntax: syntax, source: source)
+      sourceFileSyntax: syntax, source: source, ruleNameCache: ruleNameCache)
     let pipeline = LintPipeline(context: context)
     pipeline.walk(Syntax(syntax))
 

Sources/SwiftFormatCore/Context.swift

@@ -52,13 +52,17 @@ public class Context {
   /// Contains the rules have been disabled by comments for certain line numbers.
   public let ruleMask: RuleMask
 
+  /// Contains all the available rules' names associated to their types' object identifiers.
+  public let ruleNameCache: [ObjectIdentifier: String]
+
   /// Creates a new Context with the provided configuration, diagnostic engine, and file URL.
   public init(
     configuration: Configuration,
     diagnosticEngine: DiagnosticEngine?,
     fileURL: URL,
     sourceFileSyntax: SourceFileSyntax,
-    source: String? = nil
+    source: String? = nil,
+    ruleNameCache: [ObjectIdentifier: String]
   ) {
     self.configuration = configuration
     self.diagnosticEngine = diagnosticEngine
@@ -71,12 +75,22 @@ public class Context {
       syntaxNode: Syntax(sourceFileSyntax),
       sourceLocationConverter: sourceLocationConverter
     )
+    self.ruleNameCache = ruleNameCache
   }
 
   /// Given a rule's name and the node it is examining, determine if the rule is disabled at this
   /// location or not.
-  public func isRuleEnabled(_ ruleName: String, node: Syntax) -> Bool {
+  public func isRuleEnabled<R: Rule>(_ rule: R.Type, node: Syntax) -> Bool {
     let loc = node.startLocation(converter: self.sourceLocationConverter)
+
+    assert(
+      ruleNameCache[ObjectIdentifier(rule)] != nil,
+      """
+      Missing cached rule name for '\(rule)'! \
+      Ensure `generate-pipelines` has been run and `ruleNameCache` was injected.
+      """)
+
+    let ruleName = ruleNameCache[ObjectIdentifier(rule)] ?? R.ruleName
     switch ruleMask.ruleState(ruleName, at: loc) {
     case .default:
       return configuration.rules[ruleName] ?? false

Sources/SwiftFormatCore/Rule.swift

@@ -17,7 +17,7 @@ public protocol Rule {
   /// The context in which the rule is executed.
   var context: Context { get }
 
-  /// The human-readable name of the rule. This defaults to the class name.
+  /// The human-readable name of the rule. This defaults to the type name.
   static var ruleName: String { get }
 
   /// Whether this rule is opt-in, meaning it is disabled by default.
@@ -27,27 +27,7 @@ public protocol Rule {
   init(context: Context)
 }
 
-fileprivate var nameCache = [ObjectIdentifier: String]()
-fileprivate var nameCacheQueue = DispatchQueue(
-      label: "com.apple.SwiftFormat.NameCache", attributes: .concurrent)
-
 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 {
-      return cachedName
-    }
-
-    let name = String("\(self)".split(separator: ".").last!)
-    nameCacheQueue.async(flags: .barrier) {
-        nameCache[identifier] = name
-    }
-
-    return name
-  }
+  public static var ruleName: String { String("\(self)".split(separator: ".").last!) }
 }

Sources/SwiftFormatCore/SyntaxFormatRule.swift

@@ -31,7 +31,7 @@ open class SyntaxFormatRule: SyntaxRewriter, Rule {
   open override func visitAny(_ node: Syntax) -> Syntax? {
     // If the rule is not enabled, then return the node unmodified; otherwise, returning nil tells
     // SwiftSyntax to continue with the standard dispatch.
-    guard context.isRuleEnabled(Self.ruleName, node: node) else { return node }
+    guard context.isRuleEnabled(type(of: self), node: node) else { return node }
     return nil
   }
 }

Sources/SwiftFormatRules/OrderedImports.swift

@@ -324,7 +324,7 @@ fileprivate func generateLines(codeBlockItemList: CodeBlockItemListSyntax, conte
         lines.append(currentLine)
         currentLine = Line()
       }
-      let sortable = context.isRuleEnabled(OrderedImports.ruleName, node: Syntax(block))
+      let sortable = context.isRuleEnabled(OrderedImports.self, node: Syntax(block))
       currentLine.syntaxNode = .importCodeBlock(block, sortable: sortable)
     } else {
       guard let syntaxNode = currentLine.syntaxNode else {

Sources/SwiftFormatRules/RuleNameCache+Generated.swift

@@ -0,0 +1,51 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+// This file is automatically generated with generate-pipeline. Do Not Edit!
+
+/// By default, the `Rule.ruleName` should be the name of the implementing rule type.
+public let ruleNameCache: [ObjectIdentifier: String] = [
+  ObjectIdentifier(AllPublicDeclarationsHaveDocumentation.self): "AllPublicDeclarationsHaveDocumentation",
+  ObjectIdentifier(AlwaysUseLowerCamelCase.self): "AlwaysUseLowerCamelCase",
+  ObjectIdentifier(AmbiguousTrailingClosureOverload.self): "AmbiguousTrailingClosureOverload",
+  ObjectIdentifier(BeginDocumentationCommentWithOneLineSummary.self): "BeginDocumentationCommentWithOneLineSummary",
+  ObjectIdentifier(DoNotUseSemicolons.self): "DoNotUseSemicolons",
+  ObjectIdentifier(DontRepeatTypeInStaticProperties.self): "DontRepeatTypeInStaticProperties",
+  ObjectIdentifier(FileScopedDeclarationPrivacy.self): "FileScopedDeclarationPrivacy",
+  ObjectIdentifier(FullyIndirectEnum.self): "FullyIndirectEnum",
+  ObjectIdentifier(GroupNumericLiterals.self): "GroupNumericLiterals",
+  ObjectIdentifier(IdentifiersMustBeASCII.self): "IdentifiersMustBeASCII",
+  ObjectIdentifier(NeverForceUnwrap.self): "NeverForceUnwrap",
+  ObjectIdentifier(NeverUseForceTry.self): "NeverUseForceTry",
+  ObjectIdentifier(NeverUseImplicitlyUnwrappedOptionals.self): "NeverUseImplicitlyUnwrappedOptionals",
+  ObjectIdentifier(NoAccessLevelOnExtensionDeclaration.self): "NoAccessLevelOnExtensionDeclaration",
+  ObjectIdentifier(NoBlockComments.self): "NoBlockComments",
+  ObjectIdentifier(NoCasesWithOnlyFallthrough.self): "NoCasesWithOnlyFallthrough",
+  ObjectIdentifier(NoEmptyTrailingClosureParentheses.self): "NoEmptyTrailingClosureParentheses",
+  ObjectIdentifier(NoLabelsInCasePatterns.self): "NoLabelsInCasePatterns",
+  ObjectIdentifier(NoLeadingUnderscores.self): "NoLeadingUnderscores",
+  ObjectIdentifier(NoParensAroundConditions.self): "NoParensAroundConditions",
+  ObjectIdentifier(NoVoidReturnOnFunctionSignature.self): "NoVoidReturnOnFunctionSignature",
+  ObjectIdentifier(OneCasePerLine.self): "OneCasePerLine",
+  ObjectIdentifier(OneVariableDeclarationPerLine.self): "OneVariableDeclarationPerLine",
+  ObjectIdentifier(OnlyOneTrailingClosureArgument.self): "OnlyOneTrailingClosureArgument",
+  ObjectIdentifier(OrderedImports.self): "OrderedImports",
+  ObjectIdentifier(ReturnVoidInsteadOfEmptyTuple.self): "ReturnVoidInsteadOfEmptyTuple",
+  ObjectIdentifier(UseEarlyExits.self): "UseEarlyExits",
+  ObjectIdentifier(UseLetInEveryBoundCaseVariable.self): "UseLetInEveryBoundCaseVariable",
+  ObjectIdentifier(UseShorthandTypeNames.self): "UseShorthandTypeNames",
+  ObjectIdentifier(UseSingleLinePropertyGetter.self): "UseSingleLinePropertyGetter",
+  ObjectIdentifier(UseSynthesizedInitializer.self): "UseSynthesizedInitializer",
+  ObjectIdentifier(UseTripleSlashForDocumentationComments.self): "UseTripleSlashForDocumentationComments",
+  ObjectIdentifier(UseWhereClausesInForLoops.self): "UseWhereClausesInForLoops",
+  ObjectIdentifier(ValidateDocumentationComments.self): "ValidateDocumentationComments",
+]

Sources/SwiftFormatTestSupport/DiagnosingTestCase.swift

@@ -1,5 +1,6 @@
 import SwiftFormatConfiguration
 import SwiftFormatCore
+import SwiftFormatRules
 import SwiftSyntax
 import XCTest
 
@@ -39,7 +40,8 @@ open class DiagnosingTestCase: XCTestCase {
       configuration: configuration ?? Configuration(),
       diagnosticEngine: DiagnosticEngine(),
       fileURL: URL(fileURLWithPath: "/tmp/test.swift"),
-      sourceFileSyntax: sourceFileSyntax)
+      sourceFileSyntax: sourceFileSyntax,
+      ruleNameCache: ruleNameCache)
     consumer = DiagnosticTrackingConsumer()
     context.diagnosticEngine?.addConsumer(consumer)
     return context

Sources/generate-pipeline/RuleNameCacheGenerator.swift

@@ -0,0 +1,55 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import Foundation
+
+/// Generates the rule registry file used to populate the default configuration.
+final class RuleNameCacheGenerator: FileGenerator {
+
+  /// The rules collected by scanning the formatter source code.
+  let ruleCollector: RuleCollector
+
+  /// Creates a new rule registry generator.
+  init(ruleCollector: RuleCollector) {
+    self.ruleCollector = ruleCollector
+  }
+
+  func write(into handle: FileHandle) throws {
+    handle.write(
+      """
+      //===----------------------------------------------------------------------===//
+      //
+      // This source file is part of the Swift.org open source project
+      //
+      // Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+      // Licensed under Apache License v2.0 with Runtime Library Exception
+      //
+      // See https://swift.org/LICENSE.txt for license information
+      // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+      //
+      //===----------------------------------------------------------------------===//
+
+      // This file is automatically generated with generate-pipeline. Do Not Edit!
+
+      /// By default, the `Rule.ruleName` should be the name of the implementing rule type.
+      public let ruleNameCache: [ObjectIdentifier: String] = [
+
+      """
+    )
+
+    for detectedRule in ruleCollector.allLinters.sorted(by: { $0.typeName < $1.typeName }) {
+      handle.write("  ObjectIdentifier(\(detectedRule.typeName).self): \"\(detectedRule.typeName)\",\n")
+    }
+    handle.write("]\n")
+  }
+}
+

Sources/generate-pipeline/main.swift

@@ -24,6 +24,10 @@ let ruleRegistryFile = sourcesDirectory
   .appendingPathComponent("SwiftFormatConfiguration")
   .appendingPathComponent("RuleRegistry+Generated.swift")
 
+let ruleNameCacheFile = sourcesDirectory
+  .appendingPathComponent("SwiftFormatRules")
+  .appendingPathComponent("RuleNameCache+Generated.swift")
+
 var ruleCollector = RuleCollector()
 try ruleCollector.collect(from: rulesDirectory)
 
@@ -34,3 +38,7 @@ try pipelineGenerator.generateFile(at: pipelineFile)
 // Generate the rule registry dictionary for configuration.
 let registryGenerator = RuleRegistryGenerator(ruleCollector: ruleCollector)
 try registryGenerator.generateFile(at: ruleRegistryFile)
+
+// Generate the rule name cache.
+let ruleNameCacheGenerator = RuleNameCacheGenerator(ruleCollector: ruleCollector)
+try ruleNameCacheGenerator.generateFile(at: ruleNameCacheFile)

Sources/swift-format/Frontend/Frontend.swift

@@ -130,35 +130,31 @@ class Frontend {
       "processPaths(_:) should only be called when paths is non-empty.")
 
     if parallel {
-      let allFilePaths = Array(FileIterator(paths: paths))
-      DispatchQueue.concurrentPerform(iterations: allFilePaths.count) { index in
-        let path = allFilePaths[index]
-        openAndProcess(path)
+      let filesToProcess = FileIterator(paths: paths).compactMap(openAndPrepareFile)
+      DispatchQueue.concurrentPerform(iterations: filesToProcess.count) { index in
+        processFile(filesToProcess[index])
       }
     } else {
-      for path in FileIterator(paths: paths) {
-        openAndProcess(path)
-      }
+      FileIterator(paths: paths).lazy.compactMap(openAndPrepareFile).forEach(processFile)
     }
   }
 
-  /// Read and process the given path, optionally synchronizing diagnostic output.
-  private func openAndProcess(_ path: String) -> Void {
+  /// Read and prepare the file at the given path for processing, optionally synchronizing
+  /// diagnostic output.
+  private func openAndPrepareFile(atPath path: String) -> FileToProcess? {
     guard let sourceFile = FileHandle(forReadingAtPath: path) else {
       diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to open \(path)"))
-      return
+      return nil
     }
 
     guard let configuration = configuration(
       atPath: lintFormatOptions.configurationPath, orInferredFromSwiftFileAtPath: path)
     else {
       // Already diagnosed in the called method.
-      return
+      return nil
     }
 
-    let fileToProcess = FileToProcess(
-      fileHandle: sourceFile, path: path, configuration: configuration)
-    processFile(fileToProcess)
+    return FileToProcess(fileHandle: sourceFile, path: path, configuration: configuration)
   }
 
   /// Returns the configuration that applies to the given `.swift` source file, when an explicit