Skip to content

Commit

Permalink
Improve performance of collecting files to lint and lint cache lookups (
Browse files Browse the repository at this point in the history
realm#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.
  • Loading branch information
jpsim authored Nov 18, 2018
1 parent 30808b1 commit d768897
Show file tree
Hide file tree
Showing 16 changed files with 75 additions and 19 deletions.
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion Cartfile.private
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion Cartfile.resolved
Original file line number Diff line number Diff line change
@@ -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"
1 change: 1 addition & 0 deletions Carthage/Checkouts/CryptoSwift
Submodule CryptoSwift added at f2ca9c
2 changes: 1 addition & 1 deletion Carthage/Checkouts/SWXMLHash
10 changes: 5 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -83,19 +83,19 @@ 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)"

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/"

Expand Down Expand Up @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions Package.resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand All @@ -25,6 +26,7 @@ let package = Package(
.target(
name: "SwiftLintFramework",
dependencies: [
"CryptoSwift",
"SourceKittenFramework",
"Yams",
]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import CryptoSwift
import Foundation

extension Configuration {
Expand Down Expand Up @@ -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")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,21 @@ 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 {
return absolutePath.bridge().appendingPathComponent(element)
}
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? {
Expand Down
14 changes: 14 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -573,6 +576,7 @@
8F6B3153213CDCD100858E44 /* UnusedPrivateDeclarationRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnusedPrivateDeclarationRule.swift; sourceTree = "<group>"; };
8F715B82213B528B00427BD9 /* UnusedImportRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UnusedImportRule.swift; sourceTree = "<group>"; };
8F8050811FFE0CBB006F5B93 /* Configuration+IndentationStyle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Configuration+IndentationStyle.swift"; sourceTree = "<group>"; };
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 = "<group>"; };
8FC9F5101F4B8E48006826C1 /* IsDisjointRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = IsDisjointRule.swift; sourceTree = "<group>"; };
8FD216CB205584AF008ED13F /* CharacterSet+SwiftLint.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "CharacterSet+SwiftLint.swift"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -1143,13 +1148,22 @@
path = Idiomatic;
sourceTree = "<group>";
};
8FB2AE2D21A1F99200D380F3 /* Frameworks */ = {
isa = PBXGroup;
children = (
8FB2AE2E21A1F99200D380F3 /* CryptoSwift.framework */,
);
name = Frameworks;
sourceTree = "<group>";
};
D0D1210F19E87861005E4BAA = {
isa = PBXGroup;
children = (
D0D1211919E87861005E4BAA /* Products */,
D0D1211A19E87861005E4BAA /* swiftlint */,
D0D1216E19E87B05005E4BAA /* SwiftLintFramework */,
D0D1217B19E87B05005E4BAA /* SwiftLintFrameworkTests */,
8FB2AE2D21A1F99200D380F3 /* Frameworks */,
);
indentWidth = 4;
sourceTree = "<group>";
Expand Down
3 changes: 3 additions & 0 deletions SwiftLint.xcworkspace/contents.xcworkspacedata

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions SwiftLintFramework.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit d768897

Please sign in to comment.