Generated by 🚫 Danger

Periphery had a similar performance issue not long ago, and I noticed there wasn't a solid glob to regex implementation, so I ported Python's fnmatch to Swift: https://github.com/ileitch/swift-filename-matcher. It might be useful here too.

Last time we tried to speed this up, it caused some slight differences in the result of what was matched vs not, so please be super careful here.

At the moment, I'm rather concerned here that normal runs without any excludes and includes seem to become much slower sometimes.

Periphery had a similar performance issue not long ago, and I noticed there wasn't a solid glob to regex implementation, so I ported Python's fnmatch to Swift: https://github.com/ileitch/swift-filename-matcher. It might be useful here too.

This is a very helpful tip. I don't want to invent a half-backed version myself. Thanks!

Any chance this can land? 🙏

Any chance this can land? 🙏

This is a critical change that needs thorough testing. Unfortunately, I'm lacking own projects with nifty included and excluded specifications.

@JaviSoto: In case this change took effect in your projects, I'd appreciate your feedback.

@SimplyDanny, thanks for drastically improving SwfitLint's speed 💪.

On one of our projects, I noticed that - "Tuist/**/Plugin.swift" no longer excludes Tuist/Plugin.swift.

Project with ~400 files:

3.42s user 0.86s system 194% cpu 2.198 total
vs
34.77s user 8.40s system 222% cpu 19.390 total

Project with ~2000 files:

3.75s user 3.78s system 63% cpu 11.846 total
vs
31.94s user 20.20s system 133% cpu 39.146 total

On one of our projects, I noticed that - "Tuist/**/Plugin.swift" no longer excludes Tuist/Plugin.swift.

You're saying this is a regression?

Project with ~400 files:

3.42s user 0.86s system 194% cpu 2.198 total
vs
34.77s user 8.40s system 222% cpu 19.390 total

Project with ~2000 files:

3.75s user 3.78s system 63% cpu 11.846 total
vs
31.94s user 20.20s system 133% cpu 39.146 total

Down to ~10% of linting time is impressive. However, one sample with a single ** is not proof enough to merge this. Not sure how to motivate more people to test this.

You're saying this is a regression?

Yes, a little.

Down to ~10% of linting time is impressive. However, one sample with a single ** is not proof enough to merge this. Not sure how to motivate more people to test this.

It's ~15 excludes with 7-8 globs.

Also, I found https://github.com/davbeck/swift-glob which probably will be even faster with async file matching.

Running SwiftLint on a rather large enterprise codebase with lots excluded files in SwiftLint configuration:

excluded:
  - .temp
  - Dependencies
  - .swiftpm
  - "**/*/.build"
  - fastlane
  - .fastlane
  - derivedData
  - "**/Build/**/*"
  - "**/Tests/**/*"
  - "**/Mocks/**/*"
  - "**/TestUtils/**/*"

Here are the results in comparison between SwiftLint main branch in this PR:

=> SwiftLint main branch (15.0 seconds)

❯ mint run swiftlint lint . --quiet
in /development/app-xxxx-ios on master [$] took 15.0s 

=> PR #5157 (3.5 seconds)

❯ mint run swiftlint lint . --quiet
in /development/app-xxxx-ios on master [$] took 3.5s 

I appreciate your performance reports! This really helps a lot to evaluate this critical change in real-world settings.

On one of our projects, I noticed that - "Tuist/**/Plugin.swift" no longer excludes Tuist/Plugin.swift.

You're saying this is a regression?

Yes, a little.

That's now fixed with the most recent update. Thanks @nekrich and @ileitch for your efforts!

Surprisingly, when removing the last two excluded items (the ones using **) from the config, this branch ran slower.

This concerns me. @CraigSiemens: I assume the project you ran SwiftLint on is not open source, is it? I'm really interested in what causes this slow down. I don't want to make the tool slightly slower in most common settings only to improve performance in edge cases.

2.1 million Swift LOC codebase with 19 excluded patterns.

main at 8e3b50fa6

Time (mean ± σ):     61.294 s ±  0.302 s    [User: 31.682 s, System: 38.439 s]
Range (min … max):   60.934 s … 61.732 s    5 runs

This PR

Time (mean ± σ):     36.925 s ±  0.268 s    [User: 25.933 s, System: 16.765 s]
Range (min … max):   36.499 s … 37.184 s    5 runs

In summary, according to the reports, we see performance improvements down to 6% of main's runtime depending on the number of exclude patterns and the number of files they match. Other tests concluded 10%, 50% or 70%. This is impressive.

However, there are also constellations where the new approach of file matching is slightly slower. In my observations, this happens when the file system (or the file system API) has cached enough information. In this case, collecting all files is very fast, even faster than the regular expression matching. This is usually the case in performance tests and the reason why regex matching appears slower. In real word linting, these optimal conditions are not too common.

Seems like we can give this a try in the next release.

@SimplyDanny Can we cut a new release? This is a nice fix

@SimplyDanny

Unfortunately the new exclusion algorithm does not work if used with an Xcode build tool plugin that relies on a configuration file. I guess the path matching doesn't work correctly in that case.

Please note that I'm using my own build tool plugin variant (with the official SwiftLintBinary.artifactbundle.zip), which relies on a configuration file to determine which files should be linted.
The official build tool plugin relies on inputFiles of the target, which is not very useful if you want local swift packages to be linted too.

It does still work with the --use-alternative-excluding flag, which probably uses some variation of the old algorithm.

I'm not sure if you consider this a supported use case, so let me know if I should create an issue or not.

@SimplyDanny

Unfortunately the new exclusion algorithm does not work if used with an Xcode build tool plugin that relies on a configuration file. I guess the path matching doesn't work correctly in that case.

Please note that I'm using my own build tool plugin variant (with the official SwiftLintBinary.artifactbundle.zip), which relies on a configuration file to determine which files should be linted. The official build tool plugin relies on inputFiles of the target, which is not very useful if you want local swift packages to be linted too.

It does still work with the --use-alternative-excluding flag, which probably uses some variation of the old algorithm.

I'm not sure if you consider this a supported use case, so let me know if I should create an issue or not.

Everything should work as before with >=0.58.1, as I've reverted this change. Unfortunately, there are much more things to think thoroughly about before this optimization can be re-included in a future release.