Skip to content

[5.3-20201012] [completion] Filter completions on the server side #336

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 23 commits into from
Oct 19, 2020
Merged

[5.3-20201012] [completion] Filter completions on the server side #336

merged 23 commits into from
Oct 19, 2020

Conversation

benlangmuir
Copy link
Contributor

@benlangmuir benlangmuir commented Oct 16, 2020 •
edited
Loading

Cherry-pick the server-side filtering-related changes to release/5.3-20201012

rdar://69374656

benlangmuir and others added 23 commits October 16, 2020 10:54
@benlangmuir
[completion] Pass through new code-completion options
f8f98dd 
Conflicts:
	Sources/SourceKit/CodeCompletionOptions.swift
	Sources/SourceKit/SourceKitServer.swift
	Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift
@benlangmuir
[gardening] Split code-completion into its own file
e0511b9 
Similar to cursor info, there is enough code here it makes sense to
separate it out.

Conflicts:
	Sources/SourceKit/sourcekitd/CodeCompletion.swift
	Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift
@benlangmuir
[completion] Add server-side filtering UIDs
198266d
@benlangmuir
[completion] Add initial server-side filtering implementation
ffd9690 
Work in progress: add a basic implementation for keeping track of the
current completion session, and use the sourcekitd
complete.open/update/close requests.

Conflicts:
	Sources/SourceKit/sourcekitd/CodeCompletion.swift
	Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift
@benlangmuir
[completion] Set default maxResults = 200
1f28a7c
@benlangmuir
[completion] Fix tests when using server-side filtering
9d5f547 
With this change, tests pass whether we default to server-side or
client-side filtering. Also duplicate a few interesting tests to run
both ways. A future commit will beef up the test coverage for
server-side filtering specifically.

Conflicts:
	Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift
@benlangmuir
[log] Improve description of Position
34896d2 
By default, print 1-based line/column numbers using the defacto standard
line:column format. In debug print, continue to use the 0-based values
to match the constructor.
@benlangmuir
[completion] Add an extension to pass completion options per-request
b97dac9 
Conflicts:
	Sources/LanguageServerProtocol/SupportTypes/CodeCompletionOptions.swift
	Sources/SourceKit/CodeCompletionOptions.swift
	Sources/SourceKitLSP/CodeCompletionOptions.swift
@benlangmuir
[completion] Add tests for server-side filtering
4434692
@benlangmuir
[gardening] Fix copyright headers and formating in modified files
d8b5a98 
Conflicts:
	Sources/SourceKit/SourceKitServer.swift
	Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift
@benlangmuir
[completion] Add usage note for server-side filtering command-line op…
947e15d 
…tions
@benlangmuir
[gardening] Rename CodeCompletionOptions -> SKCompletionOptions
8c1ef32 
Since it is being used in the LSP protocol layer, give it a name that
clarifies that it is SourceKit-specific.
@benlangmuir
[completion] Add doc comment for CodeCompletionSession
439428e 
Conflicts:
	Sources/SourceKit/sourcekitd/CodeCompletion.swift
	Sources/SourceKit/sourcekitd/CodeCompletionSession.swift
@benlangmuir
[completion] Enable server-side filtering by default
4485191
@benlangmuir
[test] Update linuxmain
2eb8998
@benlangmuir
[buildsystem] Fix strong reference to delegate
84848df 
Typically this would create a reference cycle.
@benlangmuir
[jsonrpc] Workaround file descriptor lifetime issue
91334e5 
We should solve this in the JSONRPCConnection itself, but in the
meantime workaround by keeping pipes alive until the connection has
closed.
@benlangmuir
Shutdown toolchain connections on exit
f333b3d 
When using SourceKit-LSP in tests (or otherwise in a library), we do not
want to leak the toolchain connections.

Conflicts:
	Sources/SourceKit/sourcekitd/SwiftLanguageServer.swift
@benlangmuir
Fix merge conflict resolution errors
08f5420
@DavidGoldman @benlangmuir
Fix issue where a code-completion session at a specific offset would …
459e3b1 
…get stuck failing

- Fix for https://bugs.swift.org/browse/SR-13561 by making sure the
session close waits for the open to finish

- Add a regression test

Change-Id: Iff7217d7b03bc797e036c5329afb0765dcc1874b
@DavidGoldman @benlangmuir
Remove unneeded textDocumentChange from code-complete regression test
1667eea
@DavidGoldman @benlangmuir
Better use of XCTest for code complete regression test
cfe3e60
@benlangmuir
[sourcekit-lsp] Update spelling of completion filtering options to do…
64b2350 
…uble-dash

Change the following options to double-dash:
    --completion-max-results
    --completion-server-side-filtering

This matches the behaviour on main branch, but without pulling in the
change to use swift-argument-parser.
@benlangmuir
Copy link
Contributor Author

@swift-ci please test

@benlangmuir
Copy link
Contributor Author

  • Explanation: Code-completion server-side filtering gives a major performance improvement to completion, particularly when completing global values. For developers who import large-ish modules, this is the difference between completion being responsive and being unusably slow (several seconds per global completion).
  • Scope: Affects code-completion in sourcekit-lsp.
  • Risk: We have been using this change successfully on the main branch for a few months now, and the only impactful known issue was recently fixed and is included in this cherry-pick. If there are unanticipated regressions, command-line options can be used to revert to the old behaviour. The biggest risk is that there were many sourcekitd-related changes on main branch that we do not have on 5.3 and it is possible that in edge cases related to server shutdown we will see issues. That would most likely affect unit testing, not real users.
  • Testing: Regression tests included. Also tested manually in an editor.
  • Issue: rdar://69374656
  • Reviewer: Argyrios

@swift-ci please nominate

@svermeulen
Copy link

This is working for me. Without this, in my environment in my particular project, source kit LSP is unusably slow. With this, things seem good 👍

@benlangmuir benlangmuir merged commit fccf19f into swiftlang:release/5.3-20201012 Oct 19, 2020
@benlangmuir benlangmuir deleted the code-completion-server-side-filtering-5.3-20201012 branch October 19, 2020 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akyrtzi akyrtzi akyrtzi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@benlangmuir @svermeulen @akyrtzi @DavidGoldman