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

Refactor import and compilation directive parsing to use SwiftSyntax #92

Merged
merged 2 commits into from
Apr 25, 2020

Conversation

andrewchang-bird
Copy link
Contributor

@andrewchang-bird andrewchang-bird commented Apr 23, 2020

(Stacked on #91)

Refactor import and compilation directive parsing to use SwiftSyntax

Generator performance significantly improves by using SwiftSyntax which
translates to roughly 3x faster codegen for mid-sized projects. The goal
may be to eventually migrate everything over to SwiftSyntax, although
this will require re-implementing some of SourceKit's functionality.

  • Requires Swift 5.2 toolchain to run project and build from source
  • Refactor ParseFilesOperation into multiple operations
  • Add retainForever(_:) due to slow dealloc of SwiftFileParser

Add ability to bundle dylibs into CLI binary and load at startup

Adding SwiftSyntax to the generator requires the dylib _InternalSwiftSyntaxParser which is normally bundled as part of the Xcode toolchain. Due to instability and crashing in SwiftSyntax for Swift 5.1, we are using SwiftSyntax for Swift 5.2 and thus requires the Xcode 11.4 toolchain.

See also: realm/SwiftLint#3105

Several potential solutions I reviewed:

1. Only support running with Xcode 11.4

This is unacceptable because we want the CLI to run mostly independently of what Xcode toolchain is currently installed on the machine.

2. Provide the dylib when installing the CLI

This is a straightforward option and works out-of-the-box if the dylib is in the same directory as the CLI (due to the order of rpath expansion by dyld). However, now the CLI is no longer easily portable and we'd need to create an installation method for all supported package managers.

3. Embed the dylib into the CLI

This incurs a small complexity in the launch process but provides the most streamlined experience for developers. At a high level it involves embedding the dylib into the source and at runtime outputting it into a directory under a custom rpath.

Once all dependent dylibs exist, it's not possible to just use runtime loading calls dlopen / dlsym unless we fork SwiftSyntax to pull the symbols dynamically.

It's also not possible to call into dyld APIs to replace the loaded but unbound symbol references after the program has launched (__dyld_start has already completed).

https://opensource.apple.com/source/dyld

The best approach here seems to be to relaunch the current process as a subprocess with the same args and env.

@andrewchang-bird andrewchang-bird force-pushed the swift-syntax branch 5 times, most recently from 787d203 to de22b16 Compare April 23, 2020 11:51
Copy link
Contributor

@ryanmeisters ryanmeisters left a comment

Choose a reason for hiding this comment

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

Nice! 🎉

@andrewchang-bird andrewchang-bird force-pushed the swift-syntax branch 2 times, most recently from b778623 to 30f0771 Compare April 25, 2020 21:18
Significant improvement to generator performance from using SwiftSyntax.
Roughly 3x faster for mid-sized projects. The goal may be to eventually
migrate everything over to SwiftSyntax, although this will require
re-implementing some of SourceKit's functionality.

- Requires Swift 5.2 toolchain to run project and build from source
- Refactor `ParseFilesOperation` into multiple operations
- Add `retainForever(_:)` due to slow dealloc of `SwiftFileParser`
Adding SwiftSyntax to the generator requires the dylib
`_InternalSwiftSyntaxParser` which is normally bundled as part of the
Xcode toolchain. Due to instability and crashing in SwiftSyntax for
Swift 5.1, we are using SwiftSyntax for Swift 5.2 and thus requires the
Xcode 11.4 toolchain.

See also: realm/SwiftLint#3105

Several potential solutions I reviewed:

1. Only support running with Xcode 11.4

This is unacceptable because we want the CLI to run mostly independently
of what Xcode toolchain is currently installed on the machine.

2. Provide the dylib when installing the CLI

This is a straightforward option and works out-of-the-box if the dylib
is in the same directory as the CLI (due to the order of rpath
expansion by dyld). However, now the CLI is no longer easily portable
and we'd need to create an installation method for all supported package
managers.

3. Embed the dylib into the CLI

This incurs a small complexity in the launch process but provides the
most streamlined experience for developers. At a high level it involves
embedding the dylib into the source and at runtime outputting it into a
directory under a custom rpath.

Once all dependent dylibs exist, it's not possible to just use runtime
loading calls `dlopen` / `dlsym` unless we fork SwiftSyntax to pull the
symbols dynamically.

It's also not possible to call into dyld APIs to replace the loaded but
unbound symbol references after the program has launched (`__dyld_start`
has already completed).

https://opensource.apple.com/source/dyld

The best approach here seems to be to relaunch the current process as a
subprocess with the same args and env.
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 this pull request may close these issues.

2 participants