From d768897f1d7fcea296ffdd91dc08026cda3c3467 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Sun, 18 Nov 2018 14:39:02 -0800 Subject: [PATCH] Improve performance of collecting files to lint and lint cache lookups (#2465) Performance has gotten pretty bad for complex SwiftLint configurations like the one used for Lyft's iOS code base involving lots of files in the directories being linted, large configuration files and many nested configuration files. Two main areas were particularly ripe for improvement were: 1. Collecting which files to lint 2. Lint cache lookups ### Collecting which files to lint Improve this by: * using an NSOrderedSet to remove excluded paths instead of `Array.filter` * parallelizing calls to `filesToLint` for all paths to lint and exclude * using `FileManager.subpaths(atPath:)` instead of `enumerator(atPath:)` |Change|Before|After|Speed up| |-|-|-|-| |NSOrderedSet|2.438s|0.917s|2.659x| |Parallel Flat Map|2.438s|2.248s|1.085x| |Subpaths|0.939s|0.867s|1.083x| |**Total**|**2.438s**|**0.720s**|**3.386x**| ### Lint cache lookups By using an MD5 hash of the Configuration description from CryptoSwift as the cache key instead of instead the full description, we can drastically speed up cache lookups for projects with complex SwiftLint configurations. I think the dictionary lookup for very large string keys doesn't perform very well. --- * Speed up Configuration.lintablePaths * Improve cache lookup performance by up to 10x By using an MD5 hash of the Configuration description from CryptoSwift as the cache key instead of instead the full description. * Add changelog entries * Swift 4.0 & Linux compatibility * os(Darwin) isn't a thing * Allow warnings in pod lib lint SwiftLint supports building with Swift 4.0 to 4.2. There is no version of CryptoSwift to support both Swift 4.0 and Swift 4.2. So allow warnings for now. We'll make one more Swift 4.0 compatible release, then we'll bump the build requirements to Swift 4.2 and remove the `--allow-warnings` flag. --- .gitmodules | 3 +++ CHANGELOG.md | 7 +++++- Cartfile.private | 3 ++- Cartfile.resolved | 3 ++- Carthage/Checkouts/CryptoSwift | 1 + Carthage/Checkouts/SWXMLHash | 2 +- Makefile | 10 ++++---- Package.resolved | 9 ++++++++ Package.swift | 2 ++ .../Extensions/Configuration+Cache.swift | 3 ++- .../Configuration+LintableFiles.swift | 23 ++++++++++++------- .../Extensions/FileManager+SwiftLint.swift | 8 +++++++ SwiftLint.xcodeproj/project.pbxproj | 14 +++++++++++ .../contents.xcworkspacedata | 3 +++ SwiftLintFramework.podspec | 1 + circle.yml | 2 +- 16 files changed, 75 insertions(+), 19 deletions(-) create mode 160000 Carthage/Checkouts/CryptoSwift diff --git a/.gitmodules b/.gitmodules index f534e1db6..0fd07fdca 100644 --- a/.gitmodules +++ b/.gitmodules @@ -19,3 +19,6 @@ [submodule "Carthage/Checkouts/Yams"] path = Carthage/Checkouts/Yams url = https://github.com/jpsim/Yams.git +[submodule "Carthage/Checkouts/CryptoSwift"] + path = Carthage/Checkouts/CryptoSwift + url = https://github.com/krzyzanowskim/CryptoSwift.git diff --git a/CHANGELOG.md b/CHANGELOG.md index e8caf1347..68b5de394 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,12 @@ #### Bug Fixes -* None. +* Improve the performance of collecting which files to lint by up to 3.5x. + [JP Simard](https://github.com/jpsim) + +* Improve the performance of looking up cached lint results by up to 10x for + complex configurations. + [JP Simard](https://github.com/jpsim) ## 0.28.0: EcoBoost diff --git a/Cartfile.private b/Cartfile.private index dcfc9a323..ecefd1e79 100644 --- a/Cartfile.private +++ b/Cartfile.private @@ -1,3 +1,4 @@ github "Carthage/Commandant" ~> 0.15.0 -github "jspahrsummers/xcconfigs" ~> 0.12.0 github "jpsim/Yams" ~> 1.0.1 +github "jspahrsummers/xcconfigs" ~> 0.12.0 +github "krzyzanowskim/CryptoSwift" == 0.8.3 diff --git a/Cartfile.resolved b/Cartfile.resolved index a985bb57f..b69d105a3 100644 --- a/Cartfile.resolved +++ b/Cartfile.resolved @@ -1,7 +1,8 @@ github "Carthage/Commandant" "0.15.0" github "antitypical/Result" "4.0.0" -github "drmohundro/SWXMLHash" "4.7.1" +github "drmohundro/SWXMLHash" "4.7.4" github "jpsim/SourceKitten" "0.21.2" github "jpsim/Yams" "1.0.1" github "jspahrsummers/xcconfigs" "0.12" +github "krzyzanowskim/CryptoSwift" "0.8.3" github "scottrhoyt/SwiftyTextTable" "0.8.2" diff --git a/Carthage/Checkouts/CryptoSwift b/Carthage/Checkouts/CryptoSwift new file mode 160000 index 000000000..f2ca9c377 --- /dev/null +++ b/Carthage/Checkouts/CryptoSwift @@ -0,0 +1 @@ +Subproject commit f2ca9c377d4b44596a131e37eeb908221fb6e6be diff --git a/Carthage/Checkouts/SWXMLHash b/Carthage/Checkouts/SWXMLHash index 24698592b..b130217d8 160000 --- a/Carthage/Checkouts/SWXMLHash +++ b/Carthage/Checkouts/SWXMLHash @@ -1 +1 @@ -Subproject commit 24698592b581aef2bdbfc86ce18695708038c624 +Subproject commit b130217d81833cecd7ddf78b76d60200f1070d98 diff --git a/Makefile b/Makefile index acd3057a3..394a41494 100644 --- a/Makefile +++ b/Makefile @@ -74,7 +74,7 @@ clean: rm -f "./portable_swiftlint.zip" swift package clean -clean_xcode: clean +clean_xcode: $(BUILD_TOOL) $(XCODEFLAGS) -configuration Test clean build: @@ -83,7 +83,7 @@ build: build_with_disable_sandbox: swift build --disable-sandbox $(SWIFT_BUILD_FLAGS) -install: clean build +install: build install -d "$(BINARIES_FOLDER)" install "$(SWIFTLINT_EXECUTABLE)" "$(BINARIES_FOLDER)" @@ -91,11 +91,11 @@ uninstall: rm -rf "$(FRAMEWORKS_FOLDER)/SwiftLintFramework.framework" rm -f "$(BINARIES_FOLDER)/swiftlint" -installables: clean build +installables: build install -d "$(TEMPORARY_FOLDER)$(BINARIES_FOLDER)" install "$(SWIFTLINT_EXECUTABLE)" "$(TEMPORARY_FOLDER)$(BINARIES_FOLDER)" -prefix_install: clean build_with_disable_sandbox +prefix_install: build_with_disable_sandbox install -d "$(PREFIX)/bin/" install "$(SWIFTLINT_EXECUTABLE)" "$(PREFIX)/bin/" @@ -130,7 +130,7 @@ display_compilation_time: publish: brew update && brew bump-formula-pr --tag=$(shell git describe --tags) --revision=$(shell git rev-parse HEAD) swiftlint - pod trunk push SwiftLintFramework.podspec --swift-version=4.2 + pod trunk push SwiftLintFramework.podspec --allow-warnings --swift-version=4.2 pod trunk push SwiftLint.podspec --swift-version=4.2 get_version: diff --git a/Package.resolved b/Package.resolved index 8ec116b7e..763bef171 100644 --- a/Package.resolved +++ b/Package.resolved @@ -10,6 +10,15 @@ "version": "0.15.0" } }, + { + "package": "CryptoSwift", + "repositoryURL": "https://github.com/krzyzanowskim/CryptoSwift.git", + "state": { + "branch": null, + "revision": "f2ca9c377d4b44596a131e37eeb908221fb6e6be", + "version": "0.8.3" + } + }, { "package": "Nimble", "repositoryURL": "https://github.com/Quick/Nimble.git", diff --git a/Package.swift b/Package.swift index 6faf0a704..fc8af5766 100644 --- a/Package.swift +++ b/Package.swift @@ -11,6 +11,7 @@ let package = Package( .package(url: "https://github.com/Carthage/Commandant.git", from: "0.15.0"), .package(url: "https://github.com/jpsim/SourceKitten.git", from: "0.21.2"), .package(url: "https://github.com/jpsim/Yams.git", from: "1.0.1"), + .package(url: "https://github.com/krzyzanowskim/CryptoSwift.git", "0.8.0"..<"0.9.0"), .package(url: "https://github.com/scottrhoyt/SwiftyTextTable.git", from: "0.8.2"), ], targets: [ @@ -25,6 +26,7 @@ let package = Package( .target( name: "SwiftLintFramework", dependencies: [ + "CryptoSwift", "SourceKittenFramework", "Yams", ] diff --git a/Source/SwiftLintFramework/Extensions/Configuration+Cache.swift b/Source/SwiftLintFramework/Extensions/Configuration+Cache.swift index 43dad9518..4269cbb23 100644 --- a/Source/SwiftLintFramework/Extensions/Configuration+Cache.swift +++ b/Source/SwiftLintFramework/Extensions/Configuration+Cache.swift @@ -1,3 +1,4 @@ +import CryptoSwift import Foundation extension Configuration { @@ -45,7 +46,7 @@ extension Configuration { ] if let jsonData = try? JSONSerialization.data(withJSONObject: jsonObject), let jsonString = String(data: jsonData, encoding: .utf8) { - return jsonString + return jsonString.md5() } queuedFatalError("Could not serialize configuration for cache") } diff --git a/Source/SwiftLintFramework/Extensions/Configuration+LintableFiles.swift b/Source/SwiftLintFramework/Extensions/Configuration+LintableFiles.swift index cf1d92b5f..04ce39f91 100644 --- a/Source/SwiftLintFramework/Extensions/Configuration+LintableFiles.swift +++ b/Source/SwiftLintFramework/Extensions/Configuration+LintableFiles.swift @@ -14,15 +14,22 @@ extension Configuration { } let pathsForPath = included.isEmpty ? fileManager.filesToLint(inPath: path, rootDirectory: nil) : [] let excludedPaths = excluded - .flatMap { Glob.resolveGlob($0) } - .flatMap { - fileManager.filesToLint(inPath: $0, rootDirectory: rootPath) + .flatMap(Glob.resolveGlob) + .parallelFlatMap { + fileManager.filesToLint(inPath: $0, rootDirectory: self.rootPath) } - let includedPaths = included.flatMap { - fileManager.filesToLint(inPath: $0, rootDirectory: rootPath) - } - return (pathsForPath + includedPaths).filter { - !excludedPaths.contains($0) + let includedPaths = included.parallelFlatMap { + fileManager.filesToLint(inPath: $0, rootDirectory: self.rootPath) } +#if os(Linux) + let allPaths = pathsForPath + includedPaths + let result = NSMutableOrderedSet(capacity: allPaths.count) + result.addObjects(from: allPaths) +#else + let result = NSMutableOrderedSet(array: pathsForPath + includedPaths) +#endif + result.minusSet(Set(excludedPaths)) + // swiftlint:disable:next force_cast + return result.map { $0 as! String } } } diff --git a/Source/SwiftLintFramework/Extensions/FileManager+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/FileManager+SwiftLint.swift index e98bb1beb..6f77f85bd 100644 --- a/Source/SwiftLintFramework/Extensions/FileManager+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/FileManager+SwiftLint.swift @@ -17,6 +17,7 @@ extension FileManager: LintableFileManager { return [absolutePath] } +#if os(Linux) return enumerator(atPath: absolutePath)?.compactMap { element -> String? in if let element = element as? String, element.bridge().isSwiftFile() && (absolutePath + "/" + element).isFile { @@ -24,6 +25,13 @@ extension FileManager: LintableFileManager { } return nil } ?? [] +#else + return subpaths(atPath: absolutePath)?.compactMap { element -> String? in + guard element.bridge().isSwiftFile() else { return nil } + let absoluteElementPath = absolutePath.bridge().appendingPathComponent(element) + return absoluteElementPath.isFile ? absoluteElementPath : nil + } ?? [] +#endif } public func modificationDate(forFileAtPath path: String) -> Date? { diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 1fee6d410..b55199059 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -153,6 +153,8 @@ 8F6B3154213CDCD100858E44 /* UnusedPrivateDeclarationRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8F6B3153213CDCD100858E44 /* UnusedPrivateDeclarationRule.swift */; }; 8F715B83213B528B00427BD9 /* UnusedImportRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8F715B82213B528B00427BD9 /* UnusedImportRule.swift */; }; 8F8050821FFE0CBB006F5B93 /* Configuration+IndentationStyle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8F8050811FFE0CBB006F5B93 /* Configuration+IndentationStyle.swift */; }; + 8FB2AE2F21A1F99200D380F3 /* CryptoSwift.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 8FB2AE2E21A1F99200D380F3 /* CryptoSwift.framework */; }; + 8FB2AE3021A1F99F00D380F3 /* CryptoSwift.framework in Embed Frameworks into SwiftLintFramework.framework */ = {isa = PBXBuildFile; fileRef = 8FB2AE2E21A1F99200D380F3 /* CryptoSwift.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; 8FC8523B2117BDDE0015269B /* ExplicitSelfRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8FC8523A2117BDDE0015269B /* ExplicitSelfRule.swift */; }; 8FC9F5111F4B8E48006826C1 /* IsDisjointRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8FC9F5101F4B8E48006826C1 /* IsDisjointRule.swift */; }; 8FD216CC205584AF008ED13F /* CharacterSet+SwiftLint.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8FD216CB205584AF008ED13F /* CharacterSet+SwiftLint.swift */; }; @@ -404,6 +406,7 @@ dstPath = SwiftLintFramework.framework/Versions/Current/Frameworks; dstSubfolderSpec = 10; files = ( + 8FB2AE3021A1F99F00D380F3 /* CryptoSwift.framework in Embed Frameworks into SwiftLintFramework.framework */, 6CCFCF2A1CFEF729003239EB /* Commandant.framework in Embed Frameworks into SwiftLintFramework.framework */, 6CCFCF2C1CFEF72D003239EB /* Result.framework in Embed Frameworks into SwiftLintFramework.framework */, 6CCFCF2D1CFEF731003239EB /* SourceKittenFramework.framework in Embed Frameworks into SwiftLintFramework.framework */, @@ -573,6 +576,7 @@ 8F6B3153213CDCD100858E44 /* UnusedPrivateDeclarationRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnusedPrivateDeclarationRule.swift; sourceTree = ""; }; 8F715B82213B528B00427BD9 /* UnusedImportRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnusedImportRule.swift; sourceTree = ""; }; 8F8050811FFE0CBB006F5B93 /* Configuration+IndentationStyle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Configuration+IndentationStyle.swift"; sourceTree = ""; }; + 8FB2AE2E21A1F99200D380F3 /* CryptoSwift.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = CryptoSwift.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 8FC8523A2117BDDE0015269B /* ExplicitSelfRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExplicitSelfRule.swift; sourceTree = ""; }; 8FC9F5101F4B8E48006826C1 /* IsDisjointRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = IsDisjointRule.swift; sourceTree = ""; }; 8FD216CB205584AF008ED13F /* CharacterSet+SwiftLint.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "CharacterSet+SwiftLint.swift"; sourceTree = ""; }; @@ -827,6 +831,7 @@ isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; files = ( + 8FB2AE2F21A1F99200D380F3 /* CryptoSwift.framework in Frameworks */, E876BFBE1B07828500114ED5 /* SourceKittenFramework.framework in Frameworks */, E8C0DFCD1AD349DB007EE3D4 /* SWXMLHash.framework in Frameworks */, 3BBF2F9D1C640A0F006CD775 /* SwiftyTextTable.framework in Frameworks */, @@ -1143,6 +1148,14 @@ path = Idiomatic; sourceTree = ""; }; + 8FB2AE2D21A1F99200D380F3 /* Frameworks */ = { + isa = PBXGroup; + children = ( + 8FB2AE2E21A1F99200D380F3 /* CryptoSwift.framework */, + ); + name = Frameworks; + sourceTree = ""; + }; D0D1210F19E87861005E4BAA = { isa = PBXGroup; children = ( @@ -1150,6 +1163,7 @@ D0D1211A19E87861005E4BAA /* swiftlint */, D0D1216E19E87B05005E4BAA /* SwiftLintFramework */, D0D1217B19E87B05005E4BAA /* SwiftLintFrameworkTests */, + 8FB2AE2D21A1F99200D380F3 /* Frameworks */, ); indentWidth = 4; sourceTree = ""; diff --git a/SwiftLint.xcworkspace/contents.xcworkspacedata b/SwiftLint.xcworkspace/contents.xcworkspacedata index ea52241d9..6c6c05f4c 100644 --- a/SwiftLint.xcworkspace/contents.xcworkspacedata +++ b/SwiftLint.xcworkspace/contents.xcworkspacedata @@ -22,4 +22,7 @@ + + diff --git a/SwiftLintFramework.podspec b/SwiftLintFramework.podspec index 9d75c35d8..945a08cb5 100644 --- a/SwiftLintFramework.podspec +++ b/SwiftLintFramework.podspec @@ -9,6 +9,7 @@ Pod::Spec.new do |s| s.platform = :osx, '10.10' s.source_files = 'Source/SwiftLintFramework/**/*.swift' s.pod_target_xcconfig = { 'APPLICATION_EXTENSION_API_ONLY' => 'YES' } + s.dependency 'CryptoSwift', '0.8.3' s.dependency 'SourceKittenFramework', '~> 0.21' s.dependency 'Yams', '~> 1.0' end diff --git a/circle.yml b/circle.yml index 7b5d4d028..43ddfa699 100644 --- a/circle.yml +++ b/circle.yml @@ -79,7 +79,7 @@ jobs: - run: echo "ruby-2.4" > ~/.ruby-version - run: bundle install - run: curl -sS https://cocoapods-specs.circleci.com/fetch-cocoapods-repo-from-s3.sh | bash - - run: bundle exec pod lib lint SwiftLintFramework.podspec + - run: bundle exec pod lib lint --allow-warnings SwiftLintFramework.podspec linux_swift_4: docker: