Cache Structure Dictionaries #2922
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PaulTaykalo
added
the
discussion
PaulTaykalo
commented
Oct 26, 2019
| init(_ value: [String: SourceKitRepresentable]) { | ||
| self.value = value | ||
|
|
||
| let substructure = value["key.substructure"] as? [SourceKitRepresentable] ?? [] |
There was a problem hiding this comment.
This caching is mostly what this PR is about
Generated by 🚫 Danger |
PaulTaykalo
force-pushed
the
speedup/cache-structure-dictionaries
branch
2 times, most recently
from
f628909
to
c687c22
Compare
jpsim
approved these changes
Nov 7, 2019
Source/SwiftLintFramework/Rules/Lint/DiscardedNotificationCenterObserverRule.swift
Outdated
Show resolved
Hide resolved
Source/SwiftLintFramework/Rules/Lint/UnusedDeclarationRule.swift
Outdated
Show resolved
Hide resolved
|
Please add an entry in the "breaking changes" section of the changelog since this breaks backwards compatiblity with the API. |
PaulTaykalo
force-pushed
the
speedup/cache-structure-dictionaries
branch
2 times, most recently
from
9aa0aea
to
796cac6
Compare
PaulTaykalo
force-pushed
the
speedup/cache-structure-dictionaries
branch
from
796cac6
to
4fff698
Compare
|
Thanks for the updates and merging! Can you delete branches after you merge PRs if you don't need the branch any more? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
[String: SourceKitRepresentable]->SourceKittenDictionaryFile.structure.dictionaryWhy
substructurekey too often, and since it is a computed property, too many objects will be created and removed upon accessing itNote
This is more of a suggestion. I totally understand that this is a big change.
Some minor stylistic improvements can be done( like namings and moving extensions in and out)
i.e. this one probably should be moved to
File.structurekindshttps://github.com/realm/SwiftLint/pull/2922/files#diff-74fa1a3ab98dac161c19949597728f07R9
This solution calculates all substructures of the original dictionary. Technically, this can lead to regression in time in some cases, if
substructureis not used in rules.This is also can lead to some code duplication, in places, where it is still easier to work with
[Strings: SourekitRepresentable]BTW in order to have backward compatible API, we should probably use duplicated methods which still works with
[String: SourceKitRepresentable]and will wrap those under the hood.Please see #2924 and #2929 for the more improvements, based on this solution