Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[lldb][progress] Mitigate non-specific LLDB progress reports in Swift #8623

Merged
merged 2 commits into from
Apr 29, 2024
Merged

[lldb][progress] Mitigate non-specific LLDB progress reports in Swift #8623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84e7e51
[lldb][progress] Mitigate non-specific LLDB progress reports in Swift
chelcassanova Apr 23, 2024
21e9d75
Update test for Swift progress reports
chelcassanova Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 10 additions & 14 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2162,9 +2162,7 @@ SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module,
auto on_exit = llvm::make_scope_exit([&]() {
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
Progress("Importing Swift modules");
});
swift::ASTContext::ModuleImportKind kind) {});
});

swift::ModuleDecl *stdlib =
Expand Down Expand Up @@ -2718,9 +2716,7 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
auto on_exit = llvm::make_scope_exit([&]() {
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
Progress("Importing Swift modules");
});
swift::ASTContext::ModuleImportKind kind) {});
});

swift::ModuleDecl *stdlib =
Expand Down Expand Up @@ -3781,20 +3777,22 @@ swift::ModuleDecl *SwiftASTContext::GetModule(const SourceModule &module,

// Report progress on module importing by using a callback function in
// swift::ASTContext.
Progress progress("Importing Swift modules");
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");
Comment on lines +3780 to +3785
Copy link
Member

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?

Copy link
Author

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.

Copy link
Author

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.

switch (kind) {
case swift::ASTContext::Module:
progress.Increment(1, module_name.str());
progress->Increment(1, module_name.str());
break;
case swift::ASTContext::Overlay:
progress.Increment(1, module_name.str() + " (overlay)");
progress->Increment(1, module_name.str() + " (overlay)");
break;
case swift::ASTContext::BridgingHeader:
progress.Increment(1,
"Compiling bridging header: " + module_name.str());
progress->Increment(1, "Compiling bridging header: " +
module_name.str());
break;
}
});
Expand All @@ -3804,9 +3802,7 @@ swift::ModuleDecl *SwiftASTContext::GetModule(const SourceModule &module,
auto on_exit = llvm::make_scope_exit([&]() {
ast->SetPreModuleImportCallback(
[](llvm::StringRef module_name,
swift::ASTContext::ModuleImportKind kind) {
Progress("Importing Swift modules");
});
swift::ASTContext::ModuleImportKind kind) {});
});

// Perform the import.
Expand Down
10 changes: 10 additions & 0 deletions ...functionalities/progress_reporting/swift_progress_reporting/TestSwiftProgressReporting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,22 @@ def test_swift_progress_report(self):
"Importing Swift standard library",
]

importing_swift_reports = []
while len(beacons):
event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)
ret_args = lldb.SBDebugger.GetProgressFromEvent(event)
if self.TraceOn():
print(ret_args[0])

# When importing Swift modules, make sure that we don't get two reports
# in a row with the title "Importing Swift modules", i.e. there should be
# a report with that title followed by a report with that title and details
# attached.
if ret_args[0] == "Importing Swift modules":
next_event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)
next_ret_args = lldb.SBDebugger.GetProgressFromEvent(next_event)
self.assertRegexpMatches(next_ret_args[0], r"Importing Swift modules:+")

for beacon in beacons:
if beacon in ret_args[0]:
beacons.remove(beacon)