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

Use swift enums instead of raw values #2924

Conversation

PaulTaykalo
Copy link
Collaborator

@PaulTaykalo PaulTaykalo commented Oct 27, 2019

Since we have own dictionary, no need to have calculated properties for some parameters in dictionaries.
A lot of time was spent on conversion from stringKind of elements to some specific enum (i.e. SwiftDeclarationKind, StatementKind, SwiftExpressionKind)
image

These were pre-cached in the dictionary, which allowed to

  1. Write a bit more type-safe code
  2. Have a bit faster code (there's no need to converts Strings to enums all the time)
  3. Some dictionary subscript unneeded access removed as well.

Note

  • Some parts of code still using kind as a string because there are no enums or enum cases that represent those strings
  • If we're about to continue to use props caching in SourceKittenDictionary, we need to be careful, because if properties will take too much place in memory, SourceKittenDictionary will start behaving like a class and will be put on the heap.

@PaulTaykalo PaulTaykalo added discussion Topics that cannot be categorized as bugs or enhancements yet. They require further discussions. enhancement Ideas for improvements of existing features and rules. labels Oct 27, 2019
@SwiftLintBot
Copy link

SwiftLintBot commented Oct 27, 2019

1 Warning
⚠️ This PR may need tests.
12 Messages
📖 Linting Aerial with this PR took 1.98s vs 2.1s on master (5% faster)
📖 Linting Alamofire with this PR took 3.22s vs 3.54s on master (9% faster)
📖 Linting Firefox with this PR took 11.05s vs 11.71s on master (5% faster)
📖 Linting Kickstarter with this PR took 26.75s vs 27.91s on master (4% faster)
📖 Linting Moya with this PR took 1.97s vs 2.08s on master (5% faster)
📖 Linting Nimble with this PR took 2.45s vs 2.57s on master (4% faster)
📖 Linting Quick with this PR took 1.04s vs 1.07s on master (2% faster)
📖 Linting Realm with this PR took 3.34s vs 3.5s on master (4% faster)
📖 Linting SourceKitten with this PR took 1.62s vs 1.71s on master (5% faster)
📖 Linting Sourcery with this PR took 3.87s vs 4.15s on master (6% faster)
📖 Linting Swift with this PR took 17.81s vs 19.07s on master (6% faster)
📖 Linting WordPress with this PR took 29.93s vs 31.06s on master (3% faster)

Generated by 🚫 Danger

Copy link
Collaborator

@jpsim jpsim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 worth calling out the performance improvements in the changelog too!

@PaulTaykalo PaulTaykalo force-pushed the speedup/cache-structure-dictionaries branch 2 times, most recently from 796cac6 to 4fff698 Compare November 7, 2019 08:28
@PaulTaykalo PaulTaykalo changed the base branch from speedup/cache-structure-dictionaries to master November 7, 2019 08:56
@PaulTaykalo PaulTaykalo force-pushed the speedup/use-swift-declaration-kinds-enums-instead-of-rawValues branch from 6497350 to b1cdc11 Compare November 7, 2019 09:05
@PaulTaykalo PaulTaykalo merged commit 0db1f9c into master Nov 7, 2019
@PaulTaykalo PaulTaykalo deleted the speedup/use-swift-declaration-kinds-enums-instead-of-rawValues branch November 7, 2019 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Topics that cannot be categorized as bugs or enhancements yet. They require further discussions. enhancement Ideas for improvements of existing features and rules.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants