Skip to content

Don't filter results when opening a codecompletion session #171

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

Merged
merged 1 commit into from
Jan 18, 2022

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Jan 13, 2022

Filtering the results in SourceKit takes considerable time, which we are not really interested in measuring.

Retrieve the initial set of results without a filter text with a result limit of 200. If we need to check for an expected result, perform a subsequent code completion update that sets the filter text. Don't measure the performance of that request.

Filtering the results in SourceKit takes considerable time, which we are not really interested in measuring.

Retrieve the initial set of results without a filter text with a result limit of 200. If we need to check for an expected result, perform a subsequent code completion update that sets the filter text. Don't measure the performance of that request.
Comment on lines +257 to +258
// Set expected results to 200 to avoid inflating the time measurements by serialization time.
// To make sure that the expected result is in the results, filter by its base name.
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment is out of date


let reusingASTContext = openResponse.value.getBool(.key_ReusingASTContext)

if let expectedResult = expectedResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will be interesting to see how much slower the extra request makes it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would expect that the update should be pretty fast, but let’s see. I started a stress tester run via swiftlang/swift#37710.

@ahoppen
Copy link
Member Author

ahoppen commented Jan 17, 2022

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Jan 17, 2022

Doesn’t seem to have made a measurable runtime difference for the stress tester. Execution of https://ci.swift.org/job/swift-PR-macos-with-sourcekit-stress-tester/78/ finished in 10 hours, which matches previous runs.

@ahoppen ahoppen merged commit 8983d33 into swiftlang:main Jan 18, 2022
@ahoppen ahoppen deleted the pr/dont-filter-on-complete-open branch January 18, 2022 09:08
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