[WIP] Introduce SourcekitVariant #274

norio-nomura · 2016-10-18T02:39:21Z

Add SourceKitVariant that replacing SourceKitRepresentable.

Replace complicated SourceKitRepresentable's operations to simple operations
No performance penalty
- Same on whole data parsing
- Better on partial data parsing

func walk1(dictionay: [String: SourceKitRepresentable]) {
    if let _ = dictionay[SwiftDocKey.name.rawValue] as? String {
    }
    if let _ = dictionay[SwiftDocKey.kind.rawValue] as? String {
    }
    if let _ = (dictionay[SwiftDocKey.offset.rawValue] as? Int64).map({Int($0)}) {
    }
    if let _ = (dictionay[SwiftDocKey.length.rawValue] as? Int64).map({Int($0)}) {
    }
    guard let substructures = dictionay[SwiftDocKey.substructure.rawValue] as? [SourceKitRepresentable] else { return }
    for substructure in substructures {
        if let substructure = substructure as? [String: SourceKitRepresentable] {
            walk1(dictionay: substructure)
        }
    }
}

will be changed to:

func walk2(variant: SourceKitVariant) {
    if let _ = variant[.name]?.string {
    }
    if let _ = variant[.kind]?.string {
    }
    if let _ = variant[.offset]?.int {
    }
    if let _ = variant[.length]?.int {
    }
    guard let substructures = variant[.substructure]?.array else { return }
    for substructure in substructures {
        walk2(variant: substructure)
    }
}

jpsim · 2016-10-18T16:59:36Z

This looks pretty great! Can we wait to get this in after SPM and Linux support though? That's #267 and #268.

There's no need to keep the old versions in there, you can delete that old code 😄. I know you probably just kept it to measure the perf difference, but I'm convinced!

norio-nomura · 2016-10-19T02:58:17Z

I added FindAvailables performance tests.

testFindAvailablesWithDictionary:

[SourceKittenFrameworkTests.PerformanceTests testFindAvailablesWithDictionary]' measured [Time, seconds] average: 0.001, relative standard deviation: 25.468%, values: [0.001627, 0.001280, 0.001151, 0.000837, 0.000833, 0.001112, 0.000839, 0.000832, 0.000825, 0.000834], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100
testFindAvailablesWithVariant:

[SourceKittenFrameworkTests.PerformanceTests testFindAvailablesWithVariant]' measured [Time, seconds] average: 0.001, relative standard deviation: 209.061%, values: [0.005857, 0.000391, 0.000288, 0.000213, 0.000252, 0.000217, 0.000210, 0.000210, 0.000211, 0.000209], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100

norio-nomura · 2016-10-19T03:15:33Z

rebased.

norio-nomura · 2016-10-19T03:58:42Z

I added "RequestEditorOpen" tests.

testRequestEditorOpenWithDictionary:

'-[SourceKittenFrameworkTests.VariantPerformanceTests testRequestEditorOpenWithDictionary]' measured [Time, seconds] average: 0.021, relative standard deviation: 8.027%, values: [0.024927, 0.020399, 0.020207, 0.020037, 0.019598, 0.021164, 0.019854, 0.020056, 0.023145, 0.019551], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100
testRequestEditorOpenWithVariant:

'-[SourceKittenFrameworkTests.VariantPerformanceTests testRequestEditorOpenWithVariant]' measured [Time, seconds] average: 0.011, relative standard deviation: 8.340%, values: [0.012964, 0.010267, 0.009695, 0.010078, 0.010301, 0.010115, 0.010215, 0.010730, 0.011066, 0.010083], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100

SourceKitVariant converts SourceKit values into Swift values lazily. So, failableSend2() is faster than failableSend().
The results in #274 (comment) shows that the first access is slow on SourceKitVariant. That is trade-off to them.

I expect that overall performance will increase by SourceKitVariant because of parsing operation is 4~5 time faster than SourceKitRepresentable.

norio-nomura · 2016-10-19T06:20:31Z

Source/SourceKittenFramework/SourceKitVariant.swift

+    var nameLength: Int? { return self[.nameLength]?.int }
+    var nameOffset: Int? { return self[.nameOffset]?.int }
+    var offset: Int? { return self[.offset]?.int }
+    var substructure: [SourceKitVariant]? { return self[.substructure]?.array }


I'm considering that substructure would returns non optional.

diff --git a/Source/SourceKittenFramework/SourceKitVariant.swift b/Source/SourceKittenFramework/SourceKitVariant.swift index e8e5ee5..16641e1 100644 --- a/Source/SourceKittenFramework/SourceKitVariant.swift +++ b/Source/SourceKittenFramework/SourceKitVariant.swift @@ -332,10 +332,10 @@ extension SourceKitVariant { var nameLength: Int? { return self[.nameLength]?.int } var nameOffset: Int? { return self[.nameOffset]?.int } var offset: Int? { return self[.offset]?.int } - var substructure: [SourceKitVariant]? { return self[.substructure]?.array } - var syntaxMap: [SourceKitVariant]? { return self[.syntaxMap]?.array } + var substructure: [SourceKitVariant] { return self[.substructure]?.array ?? [] } + var syntaxMap: [SourceKitVariant] { return self[.syntaxMap]?.array ?? [] } var typeName: String? { return self[.typeName]?.string } - var inheritedtypes: [SourceKitVariant]? { return self[.inheritedtypes]?.array } + var inheritedtypes: [SourceKitVariant] { return self[.inheritedtypes]?.array ?? [] } } // MARK: - Custom

With current implementation:

variant.substructure.map { $0 in … } // $0 is [SourceKitVariant] variant.substructure?.map { $0 in … } // $0 is SourceKitVariant

will be changed to:

variant.substructure.map { $0 in … } // $0 is SourceKitVariant variant.substructure?.map { $0 in … } // invalid

Checking existence of key.substructure can be achieved by another way: variant[.substructure] != nil

I think those properties are convenience accessor, and should be more convenient.

should be more convenient.

I changed my thought.
let binding in middle of if/guard condition is useful, though ? is required.

jpsim · 2016-10-19T23:40:05Z

Source/SourceKittenFramework/SourceKitVariant.swift

+    public var offset: Int? { return self[.offset]?.int }
+    /// Substructure ([SourceKitVariant]).
+    public var substructure: [SourceKitVariant]? { return self[.substructure]?.array }
+    /// Syntax map ([SourceKitVariant]).


Maybe this could return Data or SyntaxMap instead?

jpsim · 2016-10-19T23:40:43Z

Source/SourceKittenFramework/SourceKitVariant.swift

+    public var inheritedtypes: [SourceKitVariant]? { return self[.inheritedtypes]?.array }
+}
+
+// MARK: - Accessors for SwiftDocKey


Ideally we'd just remove SwiftDocKey. Or at least just make it a private implementation detail.

jpsim · 2016-10-19T23:41:16Z

Source/SourceKittenFramework/SourceKitVariant.swift

+            switch sourcekitd_variant_get_type(sourcekitObject) {
+            case SOURCEKITD_VARIANT_TYPE_ARRAY:
+                var array = [SourceKitVariant]()
+                _ = sourcekitd_variant_array_apply(sourcekitObject) { index, value in


These might need to be adapted to work on Linux once #268 lands.

I added compatible functions on 347cc4b because I can not success to passing multiple Swift objects by using context parameter of *_apply_f()s.

Release build: > [SourceKittenFrameworkTests.PerformanceTests testFindAvailablesWithDictionary]' measured [Time, seconds] average: 0.001, relative standard deviation: 25.468%, values: [0.001627, 0.001280, 0.001151, 0.000837, 0.000833, 0.001112, 0.000839, 0.000832, 0.000825, 0.000834], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100 > [SourceKittenFrameworkTests.PerformanceTests testFindAvailablesWithVariant]' measured [Time, seconds] average: 0.001, relative standard deviation: 209.061%, values: [0.005857, 0.000391, 0.000288, 0.000213, 0.000252, 0.000217, 0.000210, 0.000210, 0.000211, 0.000209], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100

Release build: > '-[SourceKittenFrameworkTests.VariantPerformanceTests testRequestEditorOpenWithDictionary]' measured [Time, seconds] average: 0.021, relative standard deviation: 8.027%, values: [0.024927, 0.020399, 0.020207, 0.020037, 0.019598, 0.021164, 0.019854, 0.020056, 0.023145, 0.019551], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100 > '-[SourceKittenFrameworkTests.VariantPerformanceTests testRequestEditorOpenWithVariant]' measured [Time, seconds] average: 0.011, relative standard deviation: 8.340%, values: [0.012964, 0.010267, 0.009695, 0.010078, 0.010301, 0.010115, 0.010215, 0.010730, 0.011066, 0.010083], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100

…`Equatable`

It is hard to pass multiple Swift objects in context parameter on calling sourcekitd's `*_apply_f` functions. So, I added `*_apply` compatible functions that passing Swift closure as context and calling them in C function.

norio-nomura · 2016-10-20T06:29:16Z

rebased. Now this runs on Linux.

jpsim · 2016-10-20T17:49:44Z

Tests/SourceKittenFrameworkTests/VariantPerformanceTests.swift

+
+let largestSwiftFile = File(path: largestSwiftFileInRepoURL.path)!
+
+let thisFile = URL(fileURLWithPath: #file).path


isn't this equivalent to #file?

norio-nomura · 2016-10-21T14:21:42Z

I created UID that represents sourcekitd_uid_t in Swift, corrected UIdent definitions from SourceKit and added them to UID as static properties.
I think those properties would be replacement of previous enums such as SwiftDocKey.

By those changes, parsing operations become about 2 time faster than first implementation:

testFindAvailablesWithVariant:

-[SourceKittenFrameworkTests.VariantPerformanceTests testFindAvailablesWithVariant]' measured [Time, seconds] average: 0.001, relative standard deviation: 252.375%, values: [0.004816, 0.000104, 0.000089, 0.000088, 0.000087, 0.000087, 0.000087, 0.000087, 0.000087, 0.000087], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100

* master: Add workaround of #282 run DocInfoTests on Linux formatting fixes Adds support for docinfo requests. Add workaround of #280 fix String(contentsOfFile:) on Linux fix bindMemory(to:capacity:) that should be assumingMemoryBound(to:) enable 2 out of 3 SwiftDocsTests on Linux run SourceKitTests.testIndex() on Linux run CodeCompletionTests on Linux fix compareJSONString(...) on Linux to work correctly

norio-nomura · 2016-11-29T07:50:01Z

Ok, I won't.

norio-nomura · 2016-11-29T15:29:45Z

Added SourceKitObject that represents sourcekitd_object_t in Swift.

so much has changed since this review, that it no longer applies

* master: add empty changelog section release 0.15.1 update dependencies add docs to specify how SourceKit is found and linked

* master: -warn-long-function-bodies=200 Use typed closure instead of type inferred closure in `declarationsToJSON(_:)` Avoid using `+` operator for joining `[String]` and ArrayLiteral Add `display_compilation_time` target to Makefile add empty changelog entry release 0.15.2 fix CommandantSPM fixture add changelog entry fix ObjC enum cases not being documented

Reduced build times are following: `createExtensionOfUID(from:)` from 40191.6ms to 105.1ms `renderKnownUIDOf()` from 261.2ms to 36.3ms `renderEnum()` from 84.1ms to 7.0ms `renderExtension()` from 75.5ms to 7.1ms

…ekit-variant * commit 'cff312365b7d18d1f7ce043f54f4bffcbdba6e70': 0.15.3: SpeedKitten Add .DS_Store to .gitignore Set 'use legacy swift language version' to 'no' (== v3.0.1)

…ekit-variant * commit 'a014d42cd55beb06a157a063ed8d1e6b372a73e4': release 0.16.0 rename SourceKitten.podspec to SourceKittenFramework.podspec add missing Foundation import in VersionCommand.swift Automate actions around versions Update docker to `norionomura/sourcekit:302` update Yams to 0.1.4 bump Travis image to Xcode 8.2 update SWIFT_VERSION in Xcode project to '3.0' avoid empty parentheses when using trailing closures merge changelog entries Add request command terminate docstring with a period. add two trailing spaces to changelog entry Add ability to create request from yaml add empty changelog section # Conflicts: # Source/SourceKittenFramework/Request.swift

* master: enable many opt in rules and fix violations enable sorted_imports future opt-in rule enable variable_name rule and fix violations enable file_length rule and silence violations enable trailing_comma rule and fix violations enable line_length rule, set to 160 & address/silence some violations enable cyclomatic_complexity and silence one warning enable nesting rule and silence one violation refactor extensions in ClangTranslationUnit.swift enable function_body_length rule locally disable comma rule and enable rule in configuration exclude library wrappers and fixtures from SwiftLint run SwiftLint on Travis add "Run SwiftLint" build phase to sourcekitten target add .swiftlint.yml with all failing rules disabled prefer `first(where:)` over `filter().first` add empty CHANGELOG section and fix make publish # Conflicts: # Source/SourceKittenFramework/File.swift # Source/SourceKittenFramework/Request.swift # Tests/SourceKittenFrameworkTests/SourceKitTests.swift

jpsim · 2017-01-11T18:00:12Z

@norio-nomura what's the state of this? What's next here?

norio-nomura · 2017-01-12T00:13:26Z

@jpsim I think it needs to try using this in SwiftLint and confirm this would work well.
After that, I would open another PR implementing "Converting byte location/length to utf16 range as properties of SourceKitVariant" that would be expected to deprecate byte offset operations of String.

I'm not yet sure if this concept is right. Maybe it would needs to try using utf16 range properties in SwiftLint.
Sorry for slow of progress.

jpsim · 2017-01-12T00:41:13Z

No problem at all! I was just wondering if you were waiting on me for something or this was still in progress.

* master: update SWXMLHash to 3.0.4 Apply review Fix optional in String interpolation Update Yams to 0.1.5 fix doc command description use named members in a test tuple exclude clang and sourcekitd headers and module maps from Package.swift add empty changelog section release 0.17.0 remove UID map and lock update changelog entry add locks around shared global state variables improved wording in contributing.md update Text enum changelog entry Minor grammar and consistency improvements Update CHANGELOG.md Make enum case's first letter lowercase # Conflicts: # Source/SourceKittenFramework/Request.swift # Tests/SourceKittenFrameworkTests/SyntaxTests.swift

* master: (37 commits) update Yams to 0.3.1 add empty changelog section release 0.17.1 update README add changelog entry for Swift 3.1 support on macOS add docker_test_302 Make command and test on Travis ignore Package.pins Skip failing tests on Swift 3.1 for Linux revert changes to .travis.yml now that Yams 0.3 is out address SwiftLint 0.17 violations update Yams to 0.3.0 Update `CommandantSPM.json` to Swift 3.1 Change `testCommandantDocsSPM` to search Commandant's path dynamically Update formatting of `SimpleCodeCompletion.json` Update fixtures to Swift 3.1 Update testPrintEmptyStructure to Swift 3.1 Change `compareJSONString` to take `file` and `line` arguments Update fixtures of Commandant to 0.12 on Swift 3.0.2 Update Yams Use Yams from master on SwiftPM ... # Conflicts: # Carthage/Checkouts/Yams # Source/SourceKittenFramework/CodeCompletionItem.swift # Tests/SourceKittenFrameworkTests/SourceKitTests.swift # Tests/SourceKittenFrameworkTests/StructureTests.swift # Tests/SourceKittenFrameworkTests/SyntaxTests.swift

norio-nomura · 2017-04-07T03:28:32Z

As far as I see the change in 50caf4c, the way to define all known UIDs depends strongly on Swift's version, so it seems undesirable. 🤔

norio-nomura · 2017-11-05T05:23:48Z

Sorry to leave it long. 🙇
I do not feel like I can finish this anymore, and it is not my intention to prevent new efforts like #428, so I will close it.
I think that it was not good that the change around UID became too big.

norio-nomura force-pushed the nn-sourcekit-variant branch from 2700d7e to ff5611f Compare October 19, 2016 03:15

norio-nomura commented Oct 19, 2016

View reviewed changes

jpsim previously requested changes Oct 19, 2016

View reviewed changes

norio-nomura added 15 commits October 20, 2016 13:53

Add SourceKitVariant that try to replacing SourceKitRepresentable

4a9150a

Add VariantPerformanceTests

b5e80e7

Treat did as String

edcab52

Add == functions

13a36c3

Add extension for SwiftDocKey and attributes

f819bf0

Fix variable names in tests

543d175

Add testFindAvailables()

82f2545

Move VariantPerformanceTests.swift to Tests/SourceKittenFrameworkTests

2a7ffb3

Store .uid as .string

0a23691

Make SourceKitVariant conforms to ExpressibleByStringLiteral and …

c3068dc

…`Equatable`

Make convenient properties for SwiftDocKey to public

7a45891

Add __sourcekitd_variant_*_apply

347cc4b

It is hard to pass multiple Swift objects in context parameter on calling sourcekitd's `*_apply_f` functions. So, I added `*_apply` compatible functions that passing Swift closure as context and calling them in C function.

Run VariantPerformanceTests on Linux

3583c90

norio-nomura force-pushed the nn-sourcekit-variant branch from eb6e391 to 3583c90 Compare October 20, 2016 06:19

jpsim reviewed Oct 20, 2016

View reviewed changes

norio-nomura added 3 commits October 21, 2016 08:22

Remove thisFile

09b4fcf

Add UID that represents sourcekitd_uid_t

e45455e

Change SourceKitVariant._VariantCore to store uid as UID

980f51e

norio-nomura added 4 commits November 29, 2016 18:41

Add caching mechanism for sourcekitd_uid_t and String to UID

a6ceba1

Remove uidStringMap and String extensions for sourcekitd_uid_t

b6fa629

Add SourceKitObject that represents sourcekitd_object_t in Swift

93e3aee

Use SourceKitObject instead of sourcekitd_object_t

cd8aaa7

norio-nomura added 10 commits November 30, 2016 09:37

Merge branch 'master' into nn-sourcekit-variant

f0ee433

* master: add empty changelog section release 0.15.1 update dependencies add docs to specify how SourceKit is found and linked

Avoid using + operator for joining [String] and ArrayLiteral

e838e65

Reduced build times are following: `createExtensionOfUID(from:)` from 40191.6ms to 105.1ms `renderKnownUIDOf()` from 261.2ms to 36.3ms `renderEnum()` from 84.1ms to 7.0ms `renderExtension()` from 75.5ms to 7.1ms

Fix crashes on test with DEBUG configuration

0af6eec

Stop generating invalid member to UIDNamespace+generated

29b6496

Stop asserting on detecting unknown uid string

a4a1214

Merge commit 'cff312365b7d18d1f7ce043f54f4bffcbdba6e70' into nn-sourc…

2224d14

…ekit-variant * commit 'cff312365b7d18d1f7ce043f54f4bffcbdba6e70': 0.15.3: SpeedKitten Add .DS_Store to .gitignore Set 'use legacy swift language version' to 'no' (== v3.0.1)

Fix and silence SwiftLint violations

bccc7a3

norio-nomura added 4 commits February 3, 2017 10:37

Silence SwiftLint violations

874da0a

Update UIDNamespace+generated.swift to Xcode 8.3

50caf4c

eugeneego mentioned this pull request May 1, 2017

Force Unwraps #369

Open

norio-nomura closed this Nov 5, 2017

ainopara mentioned this pull request Nov 25, 2017

[WIP] Refactor Request to make it resilience. #428

Open

norio-nomura mentioned this pull request Feb 7, 2018

Add SourceKitObject that represents sourcekitd_object_t in Swift #490

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Introduce SourcekitVariant #274

[WIP] Introduce SourcekitVariant #274

norio-nomura commented Oct 18, 2016 •

edited

Loading

jpsim commented Oct 18, 2016

norio-nomura commented Oct 19, 2016

norio-nomura commented Oct 19, 2016

norio-nomura commented Oct 19, 2016

norio-nomura Oct 19, 2016 •

edited

Loading

norio-nomura Oct 19, 2016

jpsim Oct 19, 2016

jpsim Oct 19, 2016

jpsim Oct 19, 2016

norio-nomura Oct 20, 2016 •

edited

Loading

norio-nomura commented Oct 20, 2016

jpsim Oct 20, 2016

norio-nomura Oct 20, 2016

norio-nomura commented Oct 21, 2016 •

edited

Loading

norio-nomura commented Nov 29, 2016

norio-nomura commented Nov 29, 2016

jpsim commented Jan 11, 2017

norio-nomura commented Jan 12, 2017

jpsim commented Jan 12, 2017

norio-nomura commented Apr 7, 2017

norio-nomura commented Nov 5, 2017


		let largestSwiftFile = File(path: largestSwiftFileInRepoURL.path)!

		let thisFile = URL(fileURLWithPath: #file).path

[WIP] Introduce SourcekitVariant #274

[WIP] Introduce SourcekitVariant #274

Conversation

norio-nomura commented Oct 18, 2016 • edited Loading

jpsim commented Oct 18, 2016

norio-nomura commented Oct 19, 2016

norio-nomura commented Oct 19, 2016

norio-nomura commented Oct 19, 2016

norio-nomura Oct 19, 2016 • edited Loading

Choose a reason for hiding this comment

norio-nomura Oct 19, 2016

Choose a reason for hiding this comment

jpsim Oct 19, 2016

Choose a reason for hiding this comment

jpsim Oct 19, 2016

Choose a reason for hiding this comment

jpsim Oct 19, 2016

Choose a reason for hiding this comment

norio-nomura Oct 20, 2016 • edited Loading

Choose a reason for hiding this comment

norio-nomura commented Oct 20, 2016

jpsim Oct 20, 2016

Choose a reason for hiding this comment

norio-nomura Oct 20, 2016

Choose a reason for hiding this comment

norio-nomura commented Oct 21, 2016 • edited Loading

norio-nomura commented Nov 29, 2016

norio-nomura commented Nov 29, 2016

jpsim commented Jan 11, 2017

norio-nomura commented Jan 12, 2017

jpsim commented Jan 12, 2017

norio-nomura commented Apr 7, 2017

norio-nomura commented Nov 5, 2017

norio-nomura commented Oct 18, 2016 •

edited

Loading

norio-nomura Oct 19, 2016 •

edited

Loading

norio-nomura Oct 20, 2016 •

edited

Loading

norio-nomura commented Oct 21, 2016 •

edited

Loading