Skip to content

[lldb][progress] Improve Swift progress reporting #7769

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions lldb/source/Core/Progress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ Progress::~Progress() {
// Make sure to always report progress completed when this object is
// destructed so it indicates the progress dialog/activity should go away.
std::lock_guard<std::mutex> guard(m_mutex);
if (!m_completed) {
if (!m_completed)
m_completed = m_total;
ReportProgress();
}
ReportProgress();

Choose a reason for hiding this comment

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

Shouldn't this go upstream?

Choose a reason for hiding this comment

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

@chelcassanova This should go upstream, before you land this patch otherwise, you could get a merge conflict.

Choose a reason for hiding this comment

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

This depends on llvm#73605. @chelcassanova make sure to cherrypick here the merge commit after you land it upstream ;)

}

void Progress::Increment(uint64_t amount, std::string update) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,18 +472,15 @@ void SwiftLanguageRuntimeImpl::ProcessModulesToAdd() {

auto &target = m_process.GetTarget();
auto exe_module = target.GetExecutableModule();
Progress progress(
llvm::formatv("Setting up Swift reflection for '{0}'",
exe_module->GetFileSpec().GetFilename().AsCString()),
modules_to_add_snapshot.GetSize());

Progress progress("Setting up Swift reflection");
size_t completion = 0;

// Add all defered modules to reflection context that were added to
// the target since this SwiftLanguageRuntime was created.
modules_to_add_snapshot.ForEach([&](const ModuleSP &module_sp) -> bool {
AddModuleToReflectionContext(module_sp);
progress.Increment(++completion);
progress.Increment(++completion,
module_sp->GetFileSpec().GetFilename().AsCString());
return true;
});
}
Expand Down
85 changes: 65 additions & 20 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
#include "Plugins/SymbolFile/DWARF/DWARFASTParserClang.h"
#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"

#include <memory>
#include <mutex>
#include <queue>
#include <set>
Expand Down Expand Up @@ -1989,6 +1990,25 @@ SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module,
{
LLDB_SCOPED_TIMERF("%s (getStdlibModule)", m_description.c_str());
const bool can_create = true;

// Report progress on module importing by using a callback function in
// swift::ASTContext
Progress progress("Importing Swift standard library");
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[&progress](llvm::StringRef module_name, bool is_overlay) {
progress.Increment(1, (is_overlay ? module_name.str() + " (overlay)"
: module_name.str()));
});

// Clear the callback function on scope exit to prevent an out-of-scope
// access of the progress local variable
auto on_exit = llvm::make_scope_exit([&]() {
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[](llvm::StringRef module_name, bool is_overlay) {
Progress("Importing Swift modules");
});
});

swift::ModuleDecl *stdlib =
swift_ast_sp->m_ast_context_ap->getStdlibModule(can_create);
if (!stdlib || IsDWARFImported(*stdlib)) {
Expand Down Expand Up @@ -2479,6 +2499,25 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
{
LLDB_SCOPED_TIMERF("%s (getStdlibModule)", m_description.c_str());
const bool can_create = true;

// Report progress on module importing by using a callback function in
// swift::ASTContext
Progress progress("Importing Swift standard library");
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[&progress](llvm::StringRef module_name, bool is_overlay) {
progress.Increment(1, (is_overlay ? module_name.str() + " (overlay)"
: module_name.str()));
});

// Clear the callback function on scope exit to prevent an out-of-scope
// access of the progress local variable
auto on_exit = llvm::make_scope_exit([&]() {
swift_ast_sp->m_ast_context_ap->SetPreModuleImportCallback(
[](llvm::StringRef module_name, bool is_overlay) {
Progress("Importing Swift modules");
});
});

swift::ModuleDecl *stdlib =
swift_ast_sp->m_ast_context_ap->getStdlibModule(can_create);
if (!stdlib || IsDWARFImported(*stdlib)) {
Expand Down Expand Up @@ -2921,7 +2960,7 @@ swift::ASTContext *SwiftASTContext::GetASTContext() {
GetLanguageOptions(), GetTypeCheckerOptions(), GetSILOptions(),
GetSearchPathOptions(), GetClangImporterOptions(),
GetSymbolGraphOptions(), GetSourceManager(), GetDiagnosticEngine(),
/*OutputBackend=*/nullptr, ReportModuleLoadingProgress));
/*OutputBackend=*/nullptr));

if (getenv("LLDB_SWIFT_DUMP_DIAGS")) {
// NOTE: leaking a swift::PrintingDiagnosticConsumer() here, but
Expand Down Expand Up @@ -3203,15 +3242,6 @@ void SwiftASTContext::CacheModule(swift::ModuleDecl *module) {
m_swift_module_cache.insert({ID, module});
}

bool SwiftASTContext::ReportModuleLoadingProgress(llvm::StringRef module_name,
bool is_overlay) {
Progress progress(llvm::formatv(is_overlay ? "Importing overlay module {0}"
: "Importing module {0}",
module_name)
.str());
return true;
}

swift::ModuleDecl *SwiftASTContext::GetModule(const SourceModule &module,
Status &error, bool *cached) {
if (cached)
Expand Down Expand Up @@ -3262,6 +3292,24 @@ swift::ModuleDecl *SwiftASTContext::GetModule(const SourceModule &module,
// Create a diagnostic consumer for the diagnostics produced by the import.
auto import_diags = getScopedDiagnosticConsumer();

// Report progress on module importing by using a callback function in
// swift::ASTContext
Progress progress("Importing Swift modules");
ast->SetPreModuleImportCallback([&progress](llvm::StringRef module_name,
bool is_overlay) {
progress.Increment(
1, (is_overlay ? module_name.str() + " (overlay)" : module_name.str()));
});

// Clear the callback function on scope exit to prevent an out-of-scope access
// of the progress local variable
auto on_exit = llvm::make_scope_exit([&]() {
ast->SetPreModuleImportCallback(
[](llvm::StringRef module_name, bool is_overlay) {
Progress("Importing Swift modules");
});
});

// Perform the import.
swift::ModuleDecl *module_decl = ast->getModuleByName(module_basename_sref);

Expand Down Expand Up @@ -3869,10 +3917,9 @@ void SwiftASTContext::ValidateSectionModules(
Status error;

Progress progress(
llvm::formatv("Loading Swift module {0}",
llvm::formatv("Loading Swift module '{0}' dependencies",
module.GetFileSpec().GetFilename().AsCString()),
module_names.size());

size_t completion = 0;

for (const std::string &module_name : module_names) {
Expand All @@ -3881,7 +3928,7 @@ void SwiftASTContext::ValidateSectionModules(

// We have to increment the completion value even if we can't get the module
// object to stay in-sync with the total progress reporting.
progress.Increment(++completion);
progress.Increment(++completion, module_name);
if (!GetModule(module_info, error))
module.ReportWarning("unable to load swift module \"{0}\" ({1})",
module_name.c_str(), error.AsCString());
Expand Down Expand Up @@ -8318,10 +8365,7 @@ bool SwiftASTContextForExpressions::CacheUserImports(

auto src_file_imports = source_file.getImports();

Progress progress(llvm::formatv("Caching Swift user imports from '{0}'",
source_file.getFilename().data()),
src_file_imports.size());

Progress progress("Importing modules used in expression");
size_t completion = 0;

/// Find all explicit imports in the expression.
Expand All @@ -8339,7 +8383,9 @@ bool SwiftASTContextForExpressions::CacheUserImports(
source_file.walk(import_finder);

for (const auto &attributed_import : src_file_imports) {
progress.Increment(++completion);
progress.Increment(
++completion,
attributed_import.module.importedModule->getModuleFilename().str());
swift::ModuleDecl *module = attributed_import.module.importedModule;
if (module && import_finder.imports.count(module)) {
std::string module_name;
Expand Down Expand Up @@ -8456,10 +8502,9 @@ bool SwiftASTContext::GetCompileUnitImportsImpl(
llvm::formatv("Getting Swift compile unit imports for '{0}'",
compile_unit->GetPrimaryFile().GetFilename()),
cu_imports.size());

size_t completion = 0;
for (const SourceModule &module : cu_imports) {
progress.Increment(++completion);
progress.Increment(++completion, module.path.back().GetStringRef().str());
// When building the Swift stdlib with debug info these will
// show up in "Swift.o", but we already imported them and
// manually importing them will fail.
Expand Down
3 changes: 0 additions & 3 deletions lldb/source/Plugins/TypeSystem/Swift/SwiftASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,6 @@ class SwiftASTContext : public TypeSystemSwift {
swift::ModuleDecl *CreateModule(const SourceModule &module, Status &error,
swift::ImplicitImportInfo importInfo);

static bool ReportModuleLoadingProgress(llvm::StringRef module_name,
bool is_overlay);

// This function should only be called when all search paths
// for all items in a swift::ASTContext have been setup to
// allow for imports to happen correctly. Use with caution,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ def test_swift_progress_report(self):
self.runCmd("expr boo")
self.runCmd("v s")

beacons = [ "Loading Swift module",
"Caching Swift user imports from",
"Setting up Swift reflection for",
"Getting Swift compile unit imports for",
"Importing module", "Importing overlay module"]
beacons = [
"Loading Swift module",
"Importing modules used in expression",
"Setting up Swift reflection",
"Getting Swift compile unit imports for",
"Importing Swift modules",
"Importing Swift standard library",
]

while len(beacons):
event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)
Expand Down