Swift 4 + performance improvements (#8)

We ran into some serious performance improvements while using DoctorPretty for snapshot testing, so I wanted to see if there was anything I could do to fix it. I optimized a few small things (like using reduce(into:) with mutation instead of the pure version), but the real bottleneck is with the best and fits functions, which are recursive. I was able to rewrite fits as a plain loop, and that improved performance quite a bit, and we'd probably get huge improvements if we could rewrite best.
bkase · Dec 31, 2017 · d3f7348 · d3f7348
1 parent fbb4fda
commit d3f7348
Show file tree

Hide file tree

Showing 18 changed files with 98 additions and 55 deletions.
diff --git a/.swift-version b/.swift-version
@@ -1 +1 @@
-3.1
+4.0
diff --git a/.travis.yml b/.travis.yml
@@ -1,14 +1,13 @@
 os:
   - linux
   - osx
-env: 
+env:
 language: generic
 sudo: required
 dist: trusty
-osx_image: xcode8.3
+osx_image: xcode9.2
 install:
   - eval "$(curl -sL https://gist.githubusercontent.com/kylef/5c0475ff02b7c7671d2a/raw/9f442512a46d7a2af7b850d65a7e9bd31edfb09b/swiftenv-install.sh)"
 script:
   - swift build
-  - SWIFTPM_TEST_DoctorPretty=YES swift test
-
+  - swift test
diff --git a/Package.resolved b/Package.resolved
@@ -10,31 +10,40 @@
           "version": "0.2.0"
         }
       },
+      {
+        "package": "FileCheck",
+        "repositoryURL": "https://github.com/trill-lang/FileCheck.git",
+        "state": {
+          "branch": null,
+          "revision": "1c966580c83cf2e41be802992d46fe2edfc5c95f",
+          "version": "0.0.5"
+        }
+      },
       {
         "package": "Operadics",
         "repositoryURL": "https://github.com/typelift/Operadics.git",
         "state": {
           "branch": null,
-          "revision": "bc8cdd83868c395b7075d0db5e930842172012ec",
-          "version": "0.2.3"
+          "revision": "c2a14919b3653a39a9bf268c1ae0bf71ad6833fe",
+          "version": "0.3.0"
         }
       },
       {
         "package": "SwiftCheck",
         "repositoryURL": "https://github.com/typelift/SwiftCheck.git",
         "state": {
           "branch": null,
-          "revision": "97a244b4c48e5823e82f50562fc4e32b3d6507a8",
-          "version": "0.8.0"
+          "revision": "df82fb889864945c64458f38846702af729b3ee4",
+          "version": "0.9.1"
         }
       },
       {
         "package": "Swiftx",
         "repositoryURL": "https://github.com/typelift/Swiftx.git",
         "state": {
           "branch": null,
-          "revision": "32cb9a833cbc69b1ee2ebad2392eca3090b7211c",
-          "version": "0.5.3"
+          "revision": "140f1510ecb8597970c58c5a41a32bda72310d31",
+          "version": "0.6.0"
         }
       }
     ]

diff --git a/Package.swift b/Package.swift
@@ -1,21 +1,16 @@
-// swift-tools-version:3.1
+// swift-tools-version:4.0
 
 import PackageDescription
-import Foundation
-
-// HACK from https://github.com/ReactiveCocoa/ReactiveSwift/blob/master/Package.swift
-var isSwiftPMTest: Bool {
-    return ProcessInfo.processInfo.environment["SWIFTPM_TEST_DoctorPretty"] == "YES"
-}
 
 let package = Package(
     name: "DoctorPretty",
-    targets: [],
     dependencies: [
-        .Package(url: "https://github.com/typelift/Algebra.git", majorVersion: 0, minor: 2),
-        .Package(url: "https://github.com/typelift/Swiftx.git", majorVersion: 0, minor: 5)
-    ] + (isSwiftPMTest ?
-      [.Package(url: "https://github.com/typelift/SwiftCheck.git", versions: Version(0,6,0)..<Version(1,0,0))] :
-      [])
-
+        .package(url: "https://github.com/typelift/Algebra.git", .exact("0.2.0")),
+        .package(url: "https://github.com/typelift/Swiftx.git", .exact("0.6.0")),
+        .package(url: "https://github.com/typelift/SwiftCheck.git", .exact("0.9.1"))
+    ],
+    targets: [
+        .target(name: "DoctorPretty", dependencies: ["Algebra", "Swiftx"]),
+        .testTarget(name: "DoctorPrettyTests", dependencies: ["DoctorPretty", "SwiftCheck"]),
+        ]
 )
diff --git a/Sources/Alignment.swift → Sources/DoctorPretty/Alignment.swift b/Sources/Alignment.swift → Sources/DoctorPretty/Alignment.swift
diff --git a/Sources/Atoms.swift → Sources/DoctorPretty/Atoms.swift b/Sources/Atoms.swift → Sources/DoctorPretty/Atoms.swift
diff --git a/Sources/Doc.swift → Sources/DoctorPretty/Doc.swift b/Sources/Doc.swift → Sources/DoctorPretty/Doc.swift
@@ -35,7 +35,7 @@ public indirect enum Doc {
     }
 
     public static func text(_ str: String) -> Doc {
-        return str == "" ? .empty : ._text(length: str.characters.count, str)
+        return str == "" ? .empty : ._text(length: str.count, str)
     }
 
     public static var line: Doc {

diff --git a/Sources/DocFunctor.swift → Sources/DoctorPretty/DocFunctor.swift b/Sources/DocFunctor.swift → Sources/DoctorPretty/DocFunctor.swift
diff --git a/Sources/DocMonoid.swift → Sources/DoctorPretty/DocMonoid.swift b/Sources/DocMonoid.swift → Sources/DoctorPretty/DocMonoid.swift
diff --git a/Sources/Enclosed.swift → Sources/DoctorPretty/Enclosed.swift b/Sources/Enclosed.swift → Sources/DoctorPretty/Enclosed.swift
@@ -14,9 +14,9 @@ extension BidirectionalCollection where Iterator.Element == Doc, IndexDistance =
     /// Intersperses punctuation inside docs
     public func punctuate(with punctuation: Doc) -> [Doc] {
         if let d = first {
-            return [d] + self.dropFirst().reduce([Doc]()) { t in
-                let (acc, d2) = t
-                return acc <> [punctuation, d2]
+            return [d] + self.dropFirst().reduce(into: [Doc]()) { acc, d2 in
+                acc.append(punctuation)
+                acc.append(d2)
             }
         } else {
             return []
@@ -47,8 +47,8 @@ extension BidirectionalCollection where Iterator.Element == Doc, IndexDistance =
         return (
             .nest(indent,
                   left <&&> punctuated.sep()
-            ) <&&> right
-        ).grouped
+                ) <&&> right
+            ).grouped
     }
 
     /// See @enclose
@@ -66,3 +66,4 @@ extension BidirectionalCollection where Iterator.Element == Doc, IndexDistance =
         return enclose(left: Doc.lbrace, right: Doc.rbrace, separator: Doc.semi, indent: indent)
     }
 }
+
diff --git a/Sources/Extras.swift → Sources/DoctorPretty/Extras.swift b/Sources/Extras.swift → Sources/DoctorPretty/Extras.swift
@@ -9,5 +9,5 @@
 import Foundation
 
 public func spaces(_ i: Int) -> String {
-    return (0..<i).map{ _ in " " }.joined()
+    return String(repeating: " ", count: i)
 }
diff --git a/Sources/Fills.swift → Sources/DoctorPretty/Fills.swift b/Sources/Fills.swift → Sources/DoctorPretty/Fills.swift
diff --git a/Sources/HighLevelCombinators.swift → ...s/DoctorPretty/HighLevelCombinators.swift b/Sources/HighLevelCombinators.swift → ...s/DoctorPretty/HighLevelCombinators.swift
diff --git a/Sources/Monoids.swift → Sources/DoctorPretty/Monoids.swift b/Sources/Monoids.swift → Sources/DoctorPretty/Monoids.swift
diff --git a/Sources/RenderPretty.swift → Sources/DoctorPretty/RenderPretty.swift b/Sources/RenderPretty.swift → Sources/DoctorPretty/RenderPretty.swift
@@ -37,7 +37,7 @@ extension Doc {
     func renderFits(compareStrategy: @escaping CompareStrategy, ribbonFrac: Float, pageWidth: Width) -> SimpleDoc {
         let rounded = round(Float(pageWidth) * ribbonFrac)
         let ribbonChars: RibbonWidth = max(0, min(pageWidth, Width(rounded)))
-        
+
         func best(
             currNesting: IndentLevel,
             currColumn: ColumnCount,
@@ -100,19 +100,35 @@ extension Doc {
     static func nicest1(nesting: IndentLevel, column: ColumnCount, pageWidth: Width, ribbonWidth: RibbonWidth) -> (SimpleDoc, () -> SimpleDoc) -> SimpleDoc {
         return { d1, d2 in
             let wid = min(pageWidth - column, ribbonWidth - column + nesting)
-            
+
             func fits(prefix: Int, w: Int, doc: SimpleDoc) -> Bool {
-                switch (prefix, w, doc) {
-                case let (_, w, _) where w < 0: return false
-                case (_, _, .empty): return true
-                case let (m, w, .char(_, x)):
-                    return fits(prefix: m, w: w-1, doc: x)
-                case let (m, w, .text(length, _, x)):
-                    return fits(prefix: m, w: w-length, doc: x)
-                case (_, _, .line(_, _)): return true
+                var _w = w
+                var _doc = doc
+
+                while true {
+                    switch (_w, _doc) {
+                    case (_, _) where _w < 0:
+                        return false
+
+                    case (_, .empty):
+                        return true
+
+                    case let (w, .char(_, x)):
+                        _w = w - 1
+                        _doc = x
+                        continue
+
+                    case let (w, .text(length, _, x)):
+                        _w = w - length
+                        _doc = x
+                        continue
+
+                    case (_, _):
+                        return true
+                    }
                 }
             }
-            
+
             if fits(prefix: min(nesting, column), w: wid, doc: d1) {
                 return d1
             } else {

diff --git a/Sources/SimpleDoc.swift → Sources/DoctorPretty/SimpleDoc.swift b/Sources/SimpleDoc.swift → Sources/DoctorPretty/SimpleDoc.swift
diff --git a/Tests/DoctorPrettyTests/DoctorPrettySpec.swift b/Tests/DoctorPrettyTests/DoctorPrettySpec.swift
@@ -18,19 +18,19 @@ extension Doc {
     func equals(underPageWidth pageWidth: Width, doc: Doc) -> Bool {
         return
             self.renderPretty(ribbonFrac: 1.0, pageWidth: pageWidth).displayString() ==
-	        doc.renderPretty(ribbonFrac: 1.0, pageWidth: pageWidth).displayString()
+                doc.renderPretty(ribbonFrac: 1.0, pageWidth: pageWidth).displayString()
     }
 }
 
 extension Doc: Arbitrary {
     public static var arbitrary: Gen<Doc> {
-		let stringDocGen: Gen<Doc> = String.arbitrary.map{ Doc.text($0) }
-		let lineOrEmptyGen: Gen<Doc> = Gen<[Doc]>.fromElements(of: [ Doc.line, Doc.empty ])
-		return Gen<Doc>.frequency([
-		        (3, stringDocGen),
-		        (1, lineOrEmptyGen)
-		    ]).proliferate(withSize: 10)
-	          .map{ $0.cat() }
+        let stringDocGen: Gen<Doc> = String.arbitrary.map{ Doc.text($0) }
+        let lineOrEmptyGen: Gen<Doc> = Gen<Doc>.fromElements(of: [ Doc.line, Doc.empty ])
+        return Gen<Doc>.frequency([
+            (3, stringDocGen),
+            (1, lineOrEmptyGen)
+            ]).proliferate(withSize: 10)
+            .map{ $0.cat() }
     }
 }
 
@@ -67,7 +67,7 @@ class DocSpecs: XCTestCase {
         property("nesting single line is noop") <- forAll(posNum, posNum, String.arbitrary) { (x: Int, y: Int, str: String) in
             let xs = [x, y].sorted()
             let (nest, width) = (xs[0], xs[1])
-            let noNewlines = String(str.characters.filter { $0 != "\n" })
+            let noNewlines = String(str.filter { $0 != "\n" })
 
             return Doc.nest(nest, .text(noNewlines))
                 .equals(underPageWidth: width,
@@ -77,11 +77,12 @@ class DocSpecs: XCTestCase {
         property("group idempotent") <- forAll(posNum, Doc.arbitrary) { (width: Int, doc: Doc) in
             return doc.grouped
                 .equals(underPageWidth: width,
-	                    doc: doc.grouped.grouped)
+                        doc: doc.grouped.grouped)
         }
     }
 
     static var allTests = [
         ("testSpecs", testSpecs)
     ]
 }
+
diff --git a/Tests/DoctorPrettyTests/DoctorPrettyTests.swift b/Tests/DoctorPrettyTests/DoctorPrettyTests.swift
@@ -4,8 +4,8 @@ import Operadics
 
 // Most tests ported from https://github.com/minad/wl-pprint-annotated/blob/master/test/WLPPrintTests.hs
 class DoctorPrettyTests: XCTestCase {
-    func assertPretty(pageWidth: Width, str: String, doc: Doc) {
-        XCTAssertEqual(doc.renderPretty(ribbonFrac: 1.0, pageWidth: pageWidth).displayString(), str)
+    func assertPretty(pageWidth: Width, str: String, doc: Doc, file: StaticString = #file, line: UInt = #line) {
+        XCTAssertEqual(doc.renderPretty(ribbonFrac: 1.0, pageWidth: pageWidth).displayString(), str, file: file, line: line)
     }
 
     func testSimpleConstructors() {
@@ -192,6 +192,28 @@ class DoctorPrettyTests: XCTestCase {
             ].joined(separator: "\n"), doc: doc)
     }
 
+    func testLargeDocument() {
+        let doc = (1...65)
+            .map { _ in "foo" }
+            .map(Doc.text)
+            .fillSep()
+
+        XCTAssertEqual(
+            """
+            foo foo foo foo foo foo foo foo
+            foo foo foo foo foo foo foo foo
+            foo foo foo foo foo foo foo foo
+            foo foo foo foo foo foo foo foo
+            foo foo foo foo foo foo foo foo
+            foo foo foo foo foo foo foo foo
+            foo foo foo foo foo foo foo foo
+            foo foo foo foo foo foo foo foo
+            foo
+            """,
+            doc.renderPretty(ribbonFrac: 0.4, pageWidth: 80).displayString()
+        )
+    }
+
     static var allTests = [
         ("testSimpleConstructors", testSimpleConstructors),
         ("testFlatAltConstructor", testFlatAltConstructor),