From 314afd9593e91d3f9304ca6906e7cac56ff0be8b Mon Sep 17 00:00:00 2001 From: Nick Lockwood Date: Wed, 8 Aug 2018 14:39:50 +0100 Subject: [PATCH] Improved cache behavior --- Sources/CommandLine.swift | 45 +++++++++++--- ...1F44D368-293A-49E8-B611-DF6CA453D87D.plist | 60 ++++++++++++------- Tests/CommandLineTests.swift | 59 ++++++++++++++++++ Tests/PerformanceTests.swift | 27 +++++++-- format.sh | 2 +- 5 files changed, 157 insertions(+), 36 deletions(-) diff --git a/Sources/CommandLine.swift b/Sources/CommandLine.swift index 15af5b9a8..bf989b9f0 100644 --- a/Sources/CommandLine.swift +++ b/Sources/CommandLine.swift @@ -150,10 +150,14 @@ func printHelp() { """) } -private func timeEvent(block: () throws -> Void) rethrows -> String { +func timeEvent(block: () throws -> Void) rethrows -> TimeInterval { let start = CFAbsoluteTimeGetCurrent() try block() - let time = round((CFAbsoluteTimeGetCurrent() - start) * 100) / 100 // round to nearest 10ms + return CFAbsoluteTimeGetCurrent() - start +} + +private func formatTime(_ time: TimeInterval) -> String { + let time = round(time * 100) / 100 // round to nearest 10ms return String(format: "%gs", time) } @@ -304,9 +308,9 @@ func processArguments(_ args: [String], in directory: String) -> ExitCode { print("inferring swiftformat options from source file(s)...") var filesParsed = 0, formatOptions = FormatOptions.default, errors = [Error]() let fileOptions = options.fileOptions ?? .default - let time = timeEvent { + let time = formatTime(timeEvent { (filesParsed, formatOptions, errors) = inferOptions(from: inputURLs, options: fileOptions) - } + }) printWarnings(errors) if filesParsed == 0 { throw FormatError.parsing("failed to to infer options") @@ -447,7 +451,7 @@ func processArguments(_ args: [String], in directory: String) -> ExitCode { // Format the code var filesWritten = 0, filesFailed = 0, filesChecked = 0 - let time = timeEvent { + let time = formatTime(timeEvent { var _errors = [Error]() (filesWritten, filesFailed, filesChecked, _errors) = processInput(inputURLs, andWriteToOutput: outputURL, @@ -457,7 +461,7 @@ func processArguments(_ args: [String], in directory: String) -> ExitCode { dryrun: dryrun, cacheURL: cacheURL) errors += _errors - } + }) if filesWritten == 0 { if filesChecked == 0 { @@ -518,6 +522,16 @@ func inferOptions(from inputURLs: [URL], options: FileOptions) -> (Int, FormatOp return (filesParsed, inferFormatOptions(from: tokens), errors) } +func computeHash(_ source: String) -> String { + var count = 0 + var hash: UInt64 = 5381 + for byte in source.utf8 { + count += 1 + hash = 127 &* (hash & 0x00FF_FFFF_FFFF_FFFF) &+ UInt64(byte) + } + return "\(count)\(hash)" +} + func format(_ source: String, options: Options, verbose: Bool) throws -> String { // Parse source let originalTokens = tokenize(source) @@ -597,14 +611,27 @@ func processInput(_ inputURLs: [URL], if verbose { print("formatting \(inputURL.path)") } + var cacheHash: String? + var sourceHash: String? + if let cacheEntry = cache?[cacheKey], cacheEntry.hasPrefix(cachePrefix) { + cacheHash = String(cacheEntry[cachePrefix.endIndex...]) + sourceHash = computeHash(input) + } let output: String - if cache?[cacheKey] == cachePrefix + String(input.count) { + if let cacheHash = cacheHash, cacheHash == sourceHash { output = input if verbose { print("-- no changes", as: .success) } } else { output = try format(input, options: options, verbose: verbose) + if output != input { + sourceHash = nil + } + } + let cacheValue = cache.map { _ in + // Only bother computing this if cache is enabled + cachePrefix + (sourceHash ?? computeHash(output)) } if outputURL != inputURL, (try? String(contentsOf: outputURL)) != output { if !dryrun { @@ -620,7 +647,7 @@ func processInput(_ inputURLs: [URL], // No changes needed return { filesChecked += 1 - cache?[cacheKey] = cachePrefix + String(output.count) + cache?[cacheKey] = cacheValue } } if dryrun { @@ -639,7 +666,7 @@ func processInput(_ inputURLs: [URL], filesChecked += 1 filesFailed += 1 filesWritten += 1 - cache?[cacheKey] = cachePrefix + String(output.count) + cache?[cacheKey] = cacheValue } } catch { throw FormatError.writing("failed to write file \(outputURL.path), \(error)") diff --git a/SwiftFormat.xcodeproj/xcshareddata/xcbaselines/015AF2B61DC6A538008F0A8C.xcbaseline/1F44D368-293A-49E8-B611-DF6CA453D87D.plist b/SwiftFormat.xcodeproj/xcshareddata/xcbaselines/015AF2B61DC6A538008F0A8C.xcbaseline/1F44D368-293A-49E8-B611-DF6CA453D87D.plist index 9ed366aa7..5042f510c 100644 --- a/SwiftFormat.xcodeproj/xcshareddata/xcbaselines/015AF2B61DC6A538008F0A8C.xcbaseline/1F44D368-293A-49E8-B611-DF6CA453D87D.plist +++ b/SwiftFormat.xcodeproj/xcshareddata/xcbaselines/015AF2B61DC6A538008F0A8C.xcbaseline/1F44D368-293A-49E8-B611-DF6CA453D87D.plist @@ -6,14 +6,24 @@ PerformanceTests + testCachedFormatting() + + com.apple.XCTPerformanceMetric_WallClockTime + + baselineAverage + 0.022932 + baselineIntegrationDisplayName + 8 Aug 2018 at 15:43:52 + + testFormatting() com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.82129 + 0.79811 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testIndent() @@ -21,9 +31,9 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.18496 + 0.1673 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testInferring() @@ -31,9 +41,9 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.36823 + 0.41674 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testNumberFormatting() @@ -41,9 +51,9 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.033598 + 0.03429 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testRedundantSelf() @@ -51,9 +61,9 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.03929 + 0.03939 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testTokenizing() @@ -61,9 +71,19 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.58491 + 0.57043 + baselineIntegrationDisplayName + 8 Aug 2018 at 15:43:52 + + + testUncachedFormatting() + + com.apple.XCTPerformanceMetric_WallClockTime + + baselineAverage + 0.59705 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testWorstCaseFormatting() @@ -71,9 +91,9 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 2.3938 + 2.3634 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testWorstCaseIndent() @@ -81,9 +101,9 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.34182 + 0.31771 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testWorstCaseNumberFormatting() @@ -91,9 +111,9 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.042647 + 0.04353 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 testWorstCaseRedundantSelf() @@ -101,9 +121,9 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.19353 + 0.18684 baselineIntegrationDisplayName - 1 Aug 2018 at 10:40:58 + 8 Aug 2018 at 15:43:52 diff --git a/Tests/CommandLineTests.swift b/Tests/CommandLineTests.swift index 797bae29f..475a2adef 100644 --- a/Tests/CommandLineTests.swift +++ b/Tests/CommandLineTests.swift @@ -141,4 +141,63 @@ class CommandLineTests: XCTestCase { range = match.upperBound ..< range.upperBound } } + + // MARK: cache + + func testHashIsFasterThanFormatting() throws { + let sourceFile = URL(fileURLWithPath: #file) + let source = try String(contentsOf: sourceFile, encoding: .utf8) + let hash = computeHash(source + ";") + + let hashTime = timeEvent { _ = computeHash(source) == hash } + let formatTime = try timeEvent { _ = try format(source) } + XCTAssertLessThan(hashTime, formatTime) + } + + func testCacheHit() throws { + let input = "let foo = bar" + XCTAssertEqual(computeHash(input), computeHash(input)) + } + + func testCacheMiss() throws { + let input = "let foo = bar" + let output = "let foo = bar\n" + XCTAssertNotEqual(computeHash(input), computeHash(output)) + } + + func testCachePotentialFalsePositive() throws { + let input = "let foo = bar;" + let output = "let foo = bar\n" + XCTAssertNotEqual(computeHash(input), computeHash(output)) + } + + func testCachePotentialFalsePositive2() throws { + let input = """ + import Foo + import Bar + + """ + let output = """ + import Bar + import Foo + + """ + XCTAssertNotEqual(computeHash(input), computeHash(output)) + } + + // MARK: end-to-end formatting + + func testFormatting() { + CLI.print = { _, _ in } + let sourceDirectory = URL(fileURLWithPath: #file) + .deletingLastPathComponent().deletingLastPathComponent().path + + #if swift(>=4.1.5) + let args = "." + #else + let args = ". --disable redundantSelf" // redundantSelf crashes Xcode 9.4 in debug mode + #endif + + XCTAssertEqual(CLI.run(in: sourceDirectory, with: args), .ok) + } } diff --git a/Tests/PerformanceTests.swift b/Tests/PerformanceTests.swift index d55b0aaf1..e86c49a72 100644 --- a/Tests/PerformanceTests.swift +++ b/Tests/PerformanceTests.swift @@ -33,12 +33,12 @@ import SwiftFormat import XCTest class PerformanceTests: XCTestCase { + static let sourceDirectory = URL(fileURLWithPath: #file) + .deletingLastPathComponent().deletingLastPathComponent() + static let files: [String] = { var files = [String]() - let inputURL = URL(fileURLWithPath: #file) - .deletingLastPathComponent().deletingLastPathComponent() - - _ = enumerateFiles(withInputURL: inputURL) { url, _, _ in + _ = enumerateFiles(withInputURL: sourceDirectory) { url, _, _ in return { if let source = try? String(contentsOf: url) { files.append(source) @@ -63,7 +63,22 @@ class PerformanceTests: XCTestCase { let files = PerformanceTests.files let tokens = files.map { tokenize($0) } measure { - _ = tokens.map { try! format($0, rules: FormatRules.default) } + _ = tokens.map { try! format($0) } + } + } + + func testUncachedFormatting() { + CLI.print = { _, _ in } + measure { + XCTAssertEqual(CLI.run(in: PerformanceTests.sourceDirectory.path, with: ". --cache ignore"), .ok) + } + } + + func testCachedFormatting() { + CLI.print = { _, _ in } + _ = CLI.run(in: PerformanceTests.sourceDirectory.path, with: ".") // warm the cache + measure { + XCTAssertEqual(CLI.run(in: PerformanceTests.sourceDirectory.path, with: "."), .ok) } } @@ -95,7 +110,7 @@ class PerformanceTests: XCTestCase { experimentalRules: true ) measure { - _ = tokens.map { try! format($0, rules: FormatRules.default, options: options) } + _ = tokens.map { try! format($0, options: options) } } } diff --git a/format.sh b/format.sh index 930d82da5..82e8e4c66 100755 --- a/format.sh +++ b/format.sh @@ -1,3 +1,3 @@ if [[ -z "${TRAVIS}" ]]; then - CommandLineTool/swiftformat . --config .swiftformat --cache ignore + CommandLineTool/swiftformat . --cache ignore fi