Skip to content

Cache the results of SwiftASTContext::GetCompileUnitImports() #2719

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
Mar 26, 2021

Conversation

adrian-prantl
Copy link

Every time a scratch context is requested, a call to
GetCompileUnitImports() is made, which is both expensive and
redundant. This patch adds a cache that keeps track of compile units
for which the imports were already performed. This results in a
measurable performance improvement on the LLDB test suite.

rdar://75381959

This patch is much better than the original one, because it has an extra !!

@adrian-prantl
Copy link
Author

@swift-ci test

Copy link

@Teemperor Teemperor left a comment

Choose a reason for hiding this comment

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

I assume the extra ! is in if (!m_cu_imports.insert(GetCUSignature(*compile_unit)).second) ?

Every time a scratch context is requested, a call to
GetCompileUnitImports() is made, which is both expensive and
redundant. This patch adds a cache that keeps track of compile units
for which the imports were already performed. This results in a
measurable performance improvement on the LLDB test suite.

rdar://75381959
@adrian-prantl
Copy link
Author

Thanks @Teemperor !
I made it impossible to depend on the incorrect return value in this revision.

@adrian-prantl
Copy link
Author

@swift-ci test

4 similar comments
@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl adrian-prantl merged commit 162c9a8 into swiftlang:swift/main Mar 26, 2021
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