SwiftLint icon indicating copy to clipboard operation
SwiftLint copied to clipboard

Running lint without any code changes is still slow (30s vs 40s without cache)

Open davbeck opened this issue 2 years ago • 15 comments

New Issue Checklist

Describe the bug

I noticed that our SwiftLint build phase was taking a really long time to run in our project, even when no code had changed. Digging further, on my machine with our project, it was taking 30s to run without any changes. What is even stranger is that it was only taking 10s more to run without any cache at all.

Using the time profiler in instruments, I found that most of that time was taken up looking for excluded files in Glob.expandGlobstar. It appears that every excluded pattern causes SwiftLint to retrieve a recursive list of subpaths. The documentation for FileManager.subpathsOfDirectory(atPath:) calls this out as a potential performance issue.

Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint lint --quiet

Environment

  • SwiftLint version: 0.52.2 (also tested latest master)
  • Installation method used: mint (also tested from source and running the command directly)
  • Paste your configuration file:

Here is an abbreviated list of our excluded patterns:

  - ManualFrameworks
  - vendor
  - fastlane
  - SourceryOutput
  - Generated
  - templates
  - "**/.build"
  - "**/*.generated.swift"
  - "**/LocalizedStrings.swift"
  • Are you using nested configurations? No
  • Which Xcode version are you using (check xcodebuild -version)? 14.3.1 and 15.0 beta 8

I have a proof of concept that matches files based on regular expressions (converted from the glob syntax) and uses a directory enumerator to test for matches. This has a few advantages: First, the directory hierarchy can be scanned once regardless of how many include and exclude patterns are used. Second, if a directly is excluded (or not included) it can be skipped entirely no need to even scan the directory contents. We can also re-use the attributes from the enumerator to test if it is a file or folder and avoid calling fileExists(atPath:,isDirectory:).

With this change, I'm seeing the fully cached time go from 30s to 10s.

This seems to be a fairly common technique. Some scripting languages even have the glob to regex converter built in. It also appears to be what SwiftFormat does.

Before I clean this up and submit a PR, I wanted to make sure I wasn't missing anything.

davbeck avatar Sep 07 '23 01:09 davbeck

That's a major pain point that has been brought up in multiple issues already. Performance degrades when include/exclude paths containing wildcards are used in the configuration.

The current implementation in SwiftLint to iterate over all files uses plain C APIs. I'd be surprised if the directory enumerator was faster than that. But I'll be happy to learn otherwise.

The approach of replacing glob patterns with regular expressions and applying only them to the included paths to filter out excluded files is an idea that I tried in #5157. Unfortunately, OOS scans became much slower than before. I need to find some time to investigate further.

My conversion to regex might miss cases. Let's combine our two approaches. I'd be open to review and accept contributions on that front.

SimplyDanny avatar Sep 08 '23 21:09 SimplyDanny

From what I can tell, the primary issue is that the directory hierarchy is scanned (possibly multiple times, I couldn't confirm) for each include/exclude pattern, and then again for the final pass. Additionally it scans sub directories even if they are excluded (for instance, excluding the .build folder will still cause all the files inside to be scanned and then ignored), which is the main benefit of using the enumerator.

I'm mostly looking at the double star pattern here (ie **/.build) which seems to be much slower and is using the FileManager API in a pre-pass. There may be a way to get similar wins (use a single pass at the hierarchy) using the C api.

davbeck avatar Sep 08 '23 23:09 davbeck

I can confirm that SwiftLint seems to be doing some processing for every excluded: path, leading to degraded performance in large folders (like .build/), when ideally it should be able to "prune" every child path under those paths from the scan tree as soon as possible.

p4checo avatar Sep 19 '23 14:09 p4checo

It is unusable for our project. I have a very similar configuration with **/Directory with wildcard and without the excluded rule the lining takes 0.1s. With the excluded config, it takes 9.5s incrementally for just one package.

Incremental build with excluded: Pasted Graphic 6

Incremental build without excluded: Pasted Graphic 7

niorko avatar Nov 14 '23 11:11 niorko

@niorko is this with 0.54.0 that was released a few days ago?

jpsim avatar Nov 14 '23 12:11 jpsim

@niorko is this with 0.54.0 that was released a few days ago?

Yes it is with 0.54.0. But the same behavior with 0.53.0. Until it's resolved we will use it the old way (Build phase in the main target)

niorko avatar Nov 14 '23 12:11 niorko

I don’t speak for other maintainers, but personally I wouldn’t recommend using the SwiftPM plugin, for this and other reasons.

jpsim avatar Nov 14 '23 12:11 jpsim

I don’t speak for other maintainers, but personally I wouldn’t recommend using the SwiftPM plugin, for this and other reasons.

Right now our solution is not ideal, because the main target that runs the project is only a shell, everything is in SPM. So developers will see whether there are linting issues only after the main target is built, not when developing in SPM in isolation. It would be great to have working SPM integration, but I don't know any other pitfalls / issues with that.

niorko avatar Nov 14 '23 12:11 niorko

Any advancements in solving the time linting issue when using wildcard in path? It ruins the performance exponentially.

excluded:
- "**/BackendClient"

niorko avatar Jul 18 '24 10:07 niorko

Any advancements in solving the time linting issue when using wildcard in path? It ruins the performance exponentially.


excluded:

- "**/BackendClient"

#5157 is still open for testing. I'm not able to run it on a reasonably sized project with wildcarded exclude/include paths myself. So far, the feedback on it users gave was not too much, if not to say nonexistent.

SimplyDanny avatar Jul 18 '24 18:07 SimplyDanny

any update about this? i hava same experience, currently i disable it until it has better performance

dwirandyh avatar Aug 12 '24 08:08 dwirandyh

My main concern were tests and backend generated DTOs and services. I did a workaround of generating // swiftlint:disable all to make it work and it worked (by not using wildcards in exclude), but I removed the plugin, because it took so much time to prepare - almost 10 seconds for every build, which is unusable. Back to using swiftlint in the build phase in the main target.

image

niorko avatar Aug 12 '24 08:08 niorko