Skip to content

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