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

Swiftlint Analyzer uses large amounts of memory #2812

Closed
cltnschlosser opened this issue Jul 14, 2019 · 5 comments · Fixed by #2819
Closed

Swiftlint Analyzer uses large amounts of memory #2812

cltnschlosser opened this issue Jul 14, 2019 · 5 comments · Fixed by #2819

Comments

@cltnschlosser
Copy link
Contributor

New Issue Checklist

Describe the bug

Running swiflint analyzer over 4000 swift files, with both the unused import and unused private declaration rules enabled. Currently ~800 files in and memory usage is over 80GB.

I'll dig into this further later, to see if I can figure out why this is happening. Figured I'd create the issue now as a place to track the excessive memory usage.
image

Environment

  • SwiftLint version (run swiftlint version to be sure)? 0.33.1

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

  • Are you using nested configurations? No

  • Which Xcode version are you using (check xcode-select -p)? 10.2.1

@jpsim
Copy link
Collaborator

jpsim commented Jul 14, 2019

Yup. Pretty wild. Might be worth moving to an on-disk database instead of a 100% in-memory representation to reduce memory usage.

@cltnschlosser
Copy link
Contributor Author

Hey @jpsim so, I dug into this a bit and it turns out SourceKitten is leaking a lot of memory.
Root cause is anytime you call one of the sourcekitd functions that end in _create, there needs to be an associated sourcekitd_request_release called on the resulting object once we are done with it.

Take a look in the xcode memory graph debugger, you'll see leaked objects, they are named xpc_*.

I don't believe this can be fixed without breaking changes to SourceKitten. Should be fine, since SourceKitten is still 0.* versioned.

I think the best approach is to prevent the raw sourcekitd_object_t from being publicly exposed. Then wrapping them in a class which calls sourcekitd_request_release in the deinit.

I created a hacked together branch that proof of concepts this fix. Wanted to make sure I could run swiftlint analyzer without crashes and without large amounts of memory being used.
POC fix here: cltnschlosser/SourceKitten@ff56079

After this change memory usage dropped from 5GB for ~50 files to ~250MB.

I think there is still another another leak, because there are are lots of xpc strings, and memory usage was still slowly climbing. My guess is it's the keys for the dictionaries, but that's just a guess and memory usage was much more reasonable.

Also probably the root cause for: jpsim/SourceKitten#559

@jpsim
Copy link
Collaborator

jpsim commented Jul 16, 2019

Amazing work @cltnschlosser ❤️

I think the other leak is "on purpose" where we cache byte-wise lookups into String since these are operations that we need to do so frequently: https://github.com/jpsim/SourceKitten/blob/0.23.2/Source/SourceKittenFramework/String+SourceKitten.swift#L71-L78

It'd be nice to find ways to speed up those operations sufficiently that we wouldn't need to maintain an indefinite cache.

jpsim pushed a commit to jpsim/SourceKitten that referenced this issue Jul 19, 2019
I moved all code that interacts with `sourcekitd_object_t` into `SourceKitObject.swift`. This way it's easier to manage the lifecycle of `sourcekitd_object_t` pointers.

Relates to: realm/SwiftLint#2812

I think this could be simplified/improved even more by exposing SourceKitObject as a typealias for `[String: SourceKitObjectConvertible]`. Then when we make the request, converting to `sourcekitd_object_t` pointers, making the request, then freeing the pointers. This means that for the most part you're only working with swift types and the `sourcekitd_object_t` lifecycle is greatly shorted and simplified.
@cltnschlosser
Copy link
Contributor Author

I used SourceKitten master with swiftlint master to do an analyze run of 4000+ swift file codebase. Memory usage stayed below 3GB the whole time :) Also it didn't crash 👍 Much better than the 100GB+ (most of which was swapped to disk) from before.

@jpsim
Copy link
Collaborator

jpsim commented Jul 21, 2019

Nice! Looking forward to integrating that into SwiftLint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants