Skip to content

Conversation

@bnbarham
Copy link
Contributor

@bnbarham bnbarham commented May 5, 2023

Solver based results are fast within a function, where the ASTContext can be re-used. But it is significantly slower than the AST based results when outside of a function. Instead of using solver based as the primary results, only use them as a fallback for when AST based fails.

Resolves rdar://108930110.

Solver based results are fast within a function, where the `ASTContext`
can be re-used. But it is significantly slower than the AST based
results when outside of a function. Instead of using solver based as the
primary results, only use them as a fallback for when AST based fails.

Resolves rdar://108930110.
@bnbarham bnbarham requested review from ahoppen and rintaro as code owners May 5, 2023 03:53
@bnbarham
Copy link
Contributor Author

bnbarham commented May 5, 2023

@swift-ci please test

@bnbarham
Copy link
Contributor Author

bnbarham commented May 5, 2023

@swift-ci please Sourcekit stress test

@bnbarham
Copy link
Contributor Author

bnbarham commented May 5, 2023

preset=asan
@swift-ci please test with preset macOS platform

@bnbarham
Copy link
Contributor Author

bnbarham commented May 5, 2023

@swift-ci please test Linux platform

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thank you

@bnbarham bnbarham merged commit 01086bc into swiftlang:main May 5, 2023
@bnbarham bnbarham deleted the swap-cursor-info-order branch May 5, 2023 23:54
@drodriguez
Copy link
Contributor

FYI the changes in test/SourceKit/CursorInfo/static_vs_class_spelling.swift fail when compiling without assertions. I have not been able to figure out exactly where, but when trying to get the static or the class spelling in SwiftLangSupport.cpp UIdentVisitor::visitVarDecl, when calling getCorrectStaticSpelling, in the case of the compiler with assertions getParentPatternBinding returns a value, while in the without assertions case, it returns nullptr. I have been trying to find where setParentPatternBinding might not be called depending on assertions/no assertions and I cannot find it.

@bnbarham
Copy link
Contributor Author

bnbarham commented May 8, 2023

FYI the changes in test/SourceKit/CursorInfo/static_vs_class_spelling.swift fail when compiling without assertions.

That's... very strange. Is that a clean build? I don't have a release build handy, but checking the response from an actual release sourcekitd does give static as well (that's not checking main though, so it's possible something has changed).

@drodriguez
Copy link
Contributor

That's... very strange. Is that a clean build? I don't have a release build handy, but checking the response from an actual release sourcekitd does give static as well (that's not checking main though, so it's possible something has changed).

Clean builds on both the asserts and the no-asserts when I started. I needed to switch to use inproc for sourcekitd to be able to debug.

By "actual release sourcekitd" do you mean the one that shipped with some Xcode?

@bnbarham
Copy link
Contributor Author

bnbarham commented May 8, 2023

By "actual release sourcekitd" do you mean the one that shipped with some Xcode?

Yeah, I just tried on the latest release. It's possible there's something new that's causing this, it was just an extra data point.

@bnbarham
Copy link
Contributor Author

bnbarham commented May 9, 2023

Thanks @drodriguez. For now I've put up #65809 so that the test is passing. The underlying issue here is that we're not actually deserializing the PBD.

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.

4 participants