Use faster dictionaries traversing for AST Rules #2951

PaulTaykalo · 2019-11-08T22:12:51Z

No description provided.

Source/SwiftLintFramework/Protocols/ASTRule.swift

SwiftLintBot · 2019-11-08T22:51:02Z

	1 Warning
⚠️	This PR may need tests.

	12 Messages
📖	Linting Aerial with this PR took 1.28s vs 1.4s on master (8% faster)
📖	Linting Alamofire with this PR took 2.25s vs 2.72s on master (17% faster)
📖	Linting Firefox with this PR took 6.73s vs 8.15s on master (17% faster)
📖	Linting Kickstarter with this PR took 13.57s vs 16.39s on master (17% faster)
📖	Linting Moya with this PR took 1.2s vs 1.53s on master (21% faster)
📖	Linting Nimble with this PR took 1.33s vs 1.51s on master (11% faster)
📖	Linting Quick with this PR took 0.53s vs 0.55s on master (3% faster)
📖	Linting Realm with this PR took 2.16s vs 2.5s on master (13% faster)
📖	Linting SourceKitten with this PR took 0.89s vs 1.0s on master (10% faster)
📖	Linting Sourcery with this PR took 2.28s vs 3.16s on master (27% faster)
📖	Linting Swift with this PR took 11.44s vs 13.04s on master (12% faster)
📖	Linting WordPress with this PR took 14.88s vs 17.78s on master (16% faster)

Generated by 🚫 Danger

jpsim · 2019-11-08T23:05:30Z

5-15% faster across the board 👏

PaulTaykalo · 2019-11-08T23:10:47Z

@jpsim just a sec there's a another place with the same logic

…ersing dictionaries

PaulTaykalo · 2019-11-08T23:31:59Z

Oh.. there are multiple places with this kind of structure traverse

PaulTaykalo · 2019-11-08T23:35:36Z

I'm thinking about some generic traverser

PaulTaykalo · 2019-11-09T00:49:45Z

@jpsim Okay, I'm done for now.
If you have thoughts on how this traversing can be named differently - I'm all ears

Docs on traversing functions needed
Private?/public traversing functions
Conditional traversing needed to be handled (maybe this is out of this PR)
use subDict.expressionKind.map { violations(in: file} syntax over guard ?

I will not be able to fix this in few next days. So feel free to update it, or just leave comments and I'll fix those.

jpsim

Looks great! I tried seeing what it would look like to use map instead of guard as well, but prefer the guard at the end.

jpsim · 2019-11-09T21:47:32Z

CHANGELOG.md

@@ -46,6 +46,10 @@
  [PaulTaykalo](https://github.com/PaulTaykalo)
  [#2949](https://github.com/realm/SwiftLint/issues/2949)

+* Faster dictionaries traversing for AST Rules.  


Suggestion:

Speed up recursively executed rules (all AST rules and some others) by avoiding the creation of many intermediate collections when accumulating results.

jpsim · 2019-11-09T21:48:18Z

Source/SwiftLintFramework/Extensions/Dictionary+SwiftLint.swift

@@ -151,6 +151,50 @@ public struct SourceKittenDictionary {
    }
 }

+extension SourceKittenDictionary {
+    /// Traversing all substuctures of the dictionary hierarchically, calling |traverseBlock| on each node.


Nit: use markdown-style code markup using backticks instead of | so it renders nicely in Xcode.

Suggested change

/// Traversing all substuctures of the dictionary hierarchically, calling |traverseBlock| on each node.

/// Traversing all substructures of the dictionary hierarchically, calling `traverseBlock` on each node.

jpsim · 2019-11-09T21:48:38Z

Source/SwiftLintFramework/Extensions/Dictionary+SwiftLint.swift

@@ -151,6 +151,50 @@ public struct SourceKittenDictionary {
    }
 }

+extension SourceKittenDictionary {
+    /// Traversing all substuctures of the dictionary hierarchically, calling |traverseBlock| on each node.
+    /// Traversing using depth first strategy, so deepest substructures will be passed to |traversBlock| first


Typo: traversBlock -> traverseBlock

jpsim · 2019-11-09T21:49:30Z

Source/SwiftLintFramework/Extensions/Dictionary+SwiftLint.swift

+        return result
+    }
+
+    private func traverseDepthFirst<T>(collectingValuesInto array:inout [T],


Nit: space between : and inout:

Suggested change

private func traverseDepthFirst<T>(collectingValuesInto array:inout [T],

private func traverseDepthFirst<T>(collectingValuesInto array: inout [T],

jpsim · 2019-11-09T21:49:51Z

Source/SwiftLintFramework/Extensions/Dictionary+SwiftLint.swift

+    }
+
+    private func traverseWithParentDepthFirst<T>(
+        collectingValuesInto array:inout [T],


Nit: space between : and inout:

Suggested change

collectingValuesInto array:inout [T],

collectingValuesInto array: inout [T],

jpsim · 2019-11-09T22:20:30Z

Pushed this with minor edits to master because I'm cutting a release now 😄

PaulTaykalo force-pushed the speedup/faster-dictionary-traversing branch from 2a7fb4d to 1b45501 Compare November 8, 2019 22:22

jpsim approved these changes Nov 8, 2019

View reviewed changes

Source/SwiftLintFramework/Protocols/ASTRule.swift Outdated Show resolved Hide resolved

Faster dictionaries traversing for AST rules

f817351

PaulTaykalo force-pushed the speedup/faster-dictionary-traversing branch from 1b45501 to f817351 Compare November 8, 2019 22:54

Update Changelog

e54094a

Update SubstitutionCorrectableASTRule for using inout array when trav…

102a244

…ersing dictionaries

Use Generic Dictionary Traversing for multiple cases

1195f25

PaulTaykalo requested a review from jpsim November 9, 2019 00:52

PaulTaykalo added 2 commits November 9, 2019 19:29

Update documentation

c691514

Rename Traverse to Traverse with Parent

8aafd73

PaulTaykalo force-pushed the speedup/faster-dictionary-traversing branch from 1a976c7 to 8aafd73 Compare November 9, 2019 21:27

jpsim approved these changes Nov 9, 2019

View reviewed changes

jpsim closed this Nov 9, 2019

PaulTaykalo deleted the speedup/faster-dictionary-traversing branch November 10, 2019 06:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use faster dictionaries traversing for AST Rules #2951

Use faster dictionaries traversing for AST Rules #2951

PaulTaykalo commented Nov 8, 2019

SwiftLintBot commented Nov 8, 2019 •

edited

Loading

jpsim commented Nov 8, 2019

PaulTaykalo commented Nov 8, 2019 •

edited

Loading

PaulTaykalo commented Nov 8, 2019

PaulTaykalo commented Nov 8, 2019

PaulTaykalo commented Nov 9, 2019 •

edited

Loading

jpsim left a comment

jpsim Nov 9, 2019

jpsim Nov 9, 2019

jpsim Nov 9, 2019

jpsim Nov 9, 2019

jpsim Nov 9, 2019

jpsim commented Nov 9, 2019

	/// Traversing all substuctures of the dictionary hierarchically, calling \|traverseBlock\| on each node.
	/// Traversing all substructures of the dictionary hierarchically, calling `traverseBlock` on each node.

	private func traverseDepthFirst<T>(collectingValuesInto array:inout [T],
	private func traverseDepthFirst<T>(collectingValuesInto array: inout [T],

	collectingValuesInto array:inout [T],
	collectingValuesInto array: inout [T],

Use faster dictionaries traversing for AST Rules #2951

Use faster dictionaries traversing for AST Rules #2951

Conversation

PaulTaykalo commented Nov 8, 2019

SwiftLintBot commented Nov 8, 2019 • edited Loading

jpsim commented Nov 8, 2019

PaulTaykalo commented Nov 8, 2019 • edited Loading

PaulTaykalo commented Nov 8, 2019

PaulTaykalo commented Nov 8, 2019

PaulTaykalo commented Nov 9, 2019 • edited Loading

jpsim left a comment

Choose a reason for hiding this comment

jpsim Nov 9, 2019

Choose a reason for hiding this comment

jpsim Nov 9, 2019

Choose a reason for hiding this comment

jpsim Nov 9, 2019

Choose a reason for hiding this comment

jpsim Nov 9, 2019

Choose a reason for hiding this comment

jpsim Nov 9, 2019

Choose a reason for hiding this comment

jpsim commented Nov 9, 2019

SwiftLintBot commented Nov 8, 2019 •

edited

Loading

PaulTaykalo commented Nov 8, 2019 •

edited

Loading

PaulTaykalo commented Nov 9, 2019 •

edited

Loading