-
Notifications
You must be signed in to change notification settings - Fork 341
[lldb][progress] Mitigate non-specific LLDB progress reports in Swift #8623
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
[lldb][progress] Mitigate non-specific LLDB progress reports in Swift #8623
Conversation
When LLDB reports progress on importing Swift modules, it was delivering non-specific progress reports with only the title of "Importing Swift modules" and no details. To report progress on activity within Swift, LLDB first creates a progress report then updates that progress report using a callback that LLDB sets and the Swift compiler invokes when performing a full import. When LLDB triggers Swift to import modules that were already imported before, Swift will not perform a full import. Since the progress report would've already been displayed, but the callback is never invoked, this leads to the non-specific messages that were being displayed when importing Swift modules. This commit sets a unique pointer to create a progress report from within the callback function instead of creating the report before setting the callback. It also clears the callback on scope exit instead of setting it to a new progress report.
1e82295
to
84e7e51
Compare
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we not have any tests for this that need to be updated?
Just updated this to update the test. |
@swift-ci please test |
@swift-ci please test windows |
Updates the test for Swift progress reporting by checking that the "Importing Swift modules" message only appears twice on its own as the rest of the reports for this category will have details attached.
b614143
to
21e9d75
Compare
@swift-ci please test |
@swift-ci please test windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
std::unique_ptr<Progress> progress; | ||
ast->SetPreModuleImportCallback( | ||
[&progress](llvm::StringRef module_name, | ||
swift::ASTContext::ModuleImportKind kind) { | ||
if (!progress) | ||
progress = std::make_unique<Progress>("Importing Swift modules"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these callbacks always called on the same thread?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually unsure of this but I can check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked Adrian and he noted that while it's not guaranteed that the callbacks will be performed on the same thread, it is guaranteed that they will be called synchronously.
When LLDB reports progress on importing Swift modules, it was delivering non-specific progress reports with only the title of "Importing Swift modules" and no details. To report progress on activity within Swift, LLDB first creates a progress report then updates that progress report using a callback that LLDB sets and the Swift compiler invokes when performing a full import.
When LLDB triggers Swift to import modules that were already imported before, Swift will not perform a full import. Since the progress report would've already been displayed, but the callback is never invoked, this leads to the non-specific messages that were being displayed when importing Swift modules.
This commit sets a unique pointer to create a progress report from within the callback function instead of creating the report before setting the callback. It also clears the callback on scope exit instead of setting it to a new progress report.
rdar://122050052
#8572