Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excluded files impact the performance of swiftlint #5018

Open
2 tasks done
sebmarchand opened this issue May 18, 2023 · 7 comments · May be fixed by #5157
Open
2 tasks done

Excluded files impact the performance of swiftlint #5018

sebmarchand opened this issue May 18, 2023 · 7 comments · May be fixed by #5157
Labels
enhancement integration Issues related to integration of SwiftLint into toolchains.

Comments

@sebmarchand
Copy link

New Issue Checklist

Describe the bug

The number of files present in the working tree of swiftlint has a huge impact on its performance even if most of these files are ignored. Here's a concrete example/repro:

  • Create a temporary directory with just one file to lint:
mkdir /tmp/swiftlint_tests && cd /tmp/swiftlint_tests
touch test.swift
  • Use hyperfine to measure the execution time of swiftlint:
$ hyperfine --warmup 1 'swiftlint'
➜  swiftlint_tests hyperfine --warmup 1 'swiftlint'
  Time (mean ± σ):      61.4 ms ±   0.9 ms    [User: 52.2 ms, System: 7.1 ms]
  Range (min … max):    59.9 ms …  65.4 ms    46 runs
  • Create a subdirectory with a lot of swift files in it, create a linter config to exclude these files:
mkdir .build
for i in {1..10000}
do
  touch .build/$i.swift
done

echo "excluded:\n  - '**/.build'" > .swiftlint.yml
  • Make sure that these ignored files are actually ignored:
$ swiftlint
Linting Swift files in current working directory
Linting 'test.swift' (1/1)
Done linting! Found 0 violations, 0 serious in 1 file.
  • Benchmark swiftlint again:
➜  swiftlint_tests hyperfine --warmup 1 'swiftlint'
  Time (mean ± σ):     378.7 ms ±   6.1 ms    [User: 229.2 ms, System: 377.6 ms]
  Range (min … max):   368.2 ms … 389.6 ms    10 runs

The execution time is 6x what it is when these ignored files don't exist. Using the --use-alternative-excluding flag brings this to 5x but it'd still be nice to completely ignore these file. Not using a glob pattern in the config improves things by another 1x.

It looks like the code is traversing the entire file tree in a few places (e.g. in the glob implementation), we could maybe make it use a recursive approach and stop when it encounters a subdirectory that is ignored (e.g. in my example it'd not look at the content of the .build directory at all).

Environment

  • SwiftLint version (run swiftlint version to be sure)?
    From source

  • Installation method used (Homebrew, CocoaPods, building from source, etc)?
    Source

  • Paste your configuration file:
    See example

  • Are you using nested configurations?
    No

  • Which Xcode version are you using (check xcodebuild -version)?

$ xcodebuild -version
Xcode 14.3
Build version 14E222b
@keith
Copy link
Collaborator

keith commented Jul 26, 2023

We hit this case as well, it seems like for us --use-alternative-excluding fixes it, but our case isn't very complex, we just try to exclude some potential nested derived data directories

included: [Modules, tools]
excluded:
  - tools/*/.build
  - tools/*/DerivedData
  - tools/ibmodulelint/tests/fixtures
  - tools/illustrationstrip/tests/fixtures
  - tools/new_feature

Interestingly based on inspecting file opens it looks like what's happening is that when we pass a file list like @/tmp/abc.txt with just ~200 files, for every file it appears that swiftlint is re-resolving all files under the matching globs, which in the case where I noticed it is ~10k files, so I guess swiftlint is attempting ~2 million file stats before even starting to lint. Maybe that isn't the only cause of the slowdown but in my case it results in ~7 minutes to lint these ~200 files, which with --use-alternative-excluding lint in ~1 second.

You can see most of the time is spent in NSFileManager image

@SimplyDanny SimplyDanny added the integration Issues related to integration of SwiftLint into toolchains. label Jul 31, 2023
@Mordil
Copy link

Mordil commented Aug 3, 2023

Our exclusion list is as follows, and we saw a 3s->150s increase in execution time

included: # paths to include during linting.
  - '*/Sources'
  - '*/Tests'
excluded: # paths to ignore during linting. Takes precedence over `included`.
  - '.build'
  - '.swiftpm'
  - 'Sources/GraphQLQueries'
  - '**/*.graphql.swift'

@Mordil
Copy link

Mordil commented Aug 3, 2023

There doesn't seem to be a way to actually use the --use-alertnative-excluding flag when using SwiftLint as a SwiftPM plugin?

Could this "easily" be added to the configuration file?

@Mordil
Copy link

Mordil commented Aug 4, 2023

With further testing, this performance impact is only seen when using globstar exclusion patterns.

If I instead list out the directories directly, SwiftLint execution drops down to the expected sub-second timing

@yjdv
Copy link

yjdv commented Aug 16, 2023

I'm having same issue, so you solution is to change this

included: # paths to include during linting.
  - '*/Sources'
  - '*/Tests'
excluded: # paths to ignore during linting. Takes precedence over `included`.
  - '.build'
  - '.swiftpm'
  - 'Sources/GraphQLQueries'
  - 'folder/graphql.swift'

removing the ** from the path.
asking because I added few more folders to exclude and swift link plugin went from 1.6s to 23.7 seconds

@sebmarchand
Copy link
Author

In my case I've been avoiding the problem by making sure that these `.build' folders don't get generated (via some settings in the Swift VSCode extension), but that's not ideal.

@SimplyDanny
Copy link
Collaborator

As a general rule one should avoid having the include and exclude patterns match too many files. They should be rather specific to avoid performance issues. I hope this restriction can be mitigated by #5157.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement integration Issues related to integration of SwiftLint into toolchains.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants