Skip to content

Move deserialization of import search paths into the scratch context #5692

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

Merged
merged 2 commits into from
Dec 2, 2022
Merged

Move deserialization of import search paths into the scratch context #5692

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
7c592dc
Move deserialization of import search paths into the scratch context
adrian-prantl Dec 1, 2022
0a4675b
Remove the parallel warmup of SwiftASTContext (again). It's now redun…
adrian-prantl Dec 2, 2022
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
2 changes: 0 additions & 2 deletions lldb/include/lldb/Target/Target.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,6 @@ class TargetProperties : public Properties {

llvm::StringRef GetSwiftExtraClangFlags() const;

bool GetSwiftCreateModuleContextsInParallel() const;

bool GetSwiftReadMetadataFromFileCache() const;

bool GetSwiftUseReflectionSymbols() const;
Expand Down
159 changes: 73 additions & 86 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 @@ -52,10 +52,12 @@
#include "swift/Sema/IDETypeChecking.h"
#include "swift/Serialization/Validation.h"
#include "swift/SymbolGraphGen/SymbolGraphOptions.h"

#include "clang/AST/ASTContext.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/TargetOptions.h"
#include "clang/Driver/Driver.h"

#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
Expand Down Expand Up @@ -996,21 +998,6 @@ static const char *getImportFailureString(swift::serialization::Status status) {
}
}

/// Initialize the compiler invocation with it the search paths from a
/// serialized AST.
/// \returns true on success.
static bool DeserializeCompilerFlags(swift::CompilerInvocation &invocation,
StringRef section_data_ref, StringRef name,
llvm::raw_ostream &error) {
auto result = invocation.loadFromSerializedAST(section_data_ref);
if (result == swift::serialization::Status::Valid)
return true;

error << "Could not deserialize " << name << ":\n"
<< getImportFailureString(result) << "\n";
return false;
}

static void printASTValidationError(
llvm::raw_ostream &errs,
const swift::serialization::ValidationInfo &ast_info,
Expand Down Expand Up @@ -1057,6 +1044,7 @@ void SwiftASTContext::DiagnoseWarnings(Process &process, Module &module) const {
/// \returns true if an error was encountered.
static bool DeserializeAllCompilerFlags(swift::CompilerInvocation &invocation,
Module &module,
bool discover_implicit_search_paths,
const std::string &m_description,
llvm::raw_ostream &error,
bool &got_serialized_options,
Expand All @@ -1071,18 +1059,35 @@ static bool DeserializeAllCompilerFlags(swift::CompilerInvocation &invocation,
if (ast_file_datas.empty())
return false;

auto &search_path_options = invocation.getSearchPathOptions();
std::vector<std::string> import_search_paths;
llvm::StringSet<> known_import_search_paths;
for (auto &path : search_path_options.getImportSearchPaths()) {
import_search_paths.push_back(path);
known_import_search_paths.insert(path);
}

std::vector<swift::SearchPathOptions::FrameworkSearchPath>
framework_search_paths;
llvm::StringSet<> known_framework_search_paths;
for (auto &path : search_path_options.getFrameworkSearchPaths()) {
framework_search_paths.push_back(path);
known_framework_search_paths.insert(path.Path);
}

// An AST section consists of one or more AST modules, optionally
// with headers. Iterate over all AST modules.
for (auto ast_file_data_sp : ast_file_datas) {
llvm::StringRef buf((const char *)ast_file_data_sp->GetBytes(),
ast_file_data_sp->GetByteSize());
swift::serialization::ValidationInfo info;
for (; !buf.empty(); buf = buf.substr(info.bytes)) {
llvm::SmallVector<swift::serialization::SearchPath> searchPaths;
swift::serialization::ExtendedValidationInfo extended_validation_info;
info = swift::serialization::validateSerializedAST(
buf, invocation.getSILOptions().EnableOSSAModules,
/*requiredSDK*/ StringRef(), /*requiresRevisionMatch*/ false,
&extended_validation_info);
&extended_validation_info, /*dependencies*/ nullptr, &searchPaths);
bool invalid_ast = info.status != swift::serialization::Status::Valid;
bool invalid_size = (info.bytes == 0) || (info.bytes > buf.size());
bool invalid_name = info.name.empty();
Expand All @@ -1099,15 +1104,54 @@ static bool DeserializeAllCompilerFlags(swift::CompilerInvocation &invocation,

found_swift_modules = true;
StringRef moduleData = buf.substr(0, info.bytes);
got_serialized_options |=
DeserializeCompilerFlags(invocation, moduleData, info.name, error);

auto remap = [&](std::string path) {
ConstString remapped;
if (module.GetSourceMappingList().RemapPath(ConstString(path),
remapped))
return remapped.GetStringRef().str();
return path;
};

/// Initialize the compiler invocation with it the search paths from a
/// serialized AST.
auto deserializeCompilerFlags = [&]() -> bool {
auto result = invocation.loadFromSerializedAST(moduleData);
if (result == swift::serialization::Status::Valid) {
if (discover_implicit_search_paths) {
for (auto &searchPath : searchPaths) {
std::string path = remap(searchPath.Path);
if (!searchPath.IsFramework) {
if (known_import_search_paths.insert(path).second)
import_search_paths.push_back(path);
} else {
swift::SearchPathOptions::FrameworkSearchPath
framework_search_path(path, searchPath.IsSystem);
if (known_framework_search_paths.insert(path).second)
framework_search_paths.push_back(framework_search_path);
}
}
}
return true;
}

error << "Could not deserialize " << info.name << ":\n"
<< getImportFailureString(result) << "\n";
return false;
};

got_serialized_options |= deserializeCompilerFlags();

LOG_PRINTF(
GetLog(LLDBLog::Types), "SDK path from module \"%s\" was \"%s\".",
info.name.str().c_str(), invocation.getSDKPath().str().c_str());
// We will deduce a matching SDK path from DWARF later.
invocation.setSDKPath("");
}
}

search_path_options.setImportSearchPaths(import_search_paths);
search_path_options.setFrameworkSearchPaths(framework_search_paths);
return found_validation_errors;
}

Expand Down Expand Up @@ -1503,9 +1547,12 @@ SwiftASTContext::CreateInstance(lldb::LanguageType language, Module &module,
bool got_serialized_options = false;
llvm::SmallString<0> error;
llvm::raw_svector_ostream errs(error);
if (DeserializeAllCompilerFlags(
swift_ast_sp->GetCompilerInvocation(), module, m_description, errs,
got_serialized_options, found_swift_modules)) {
// Implicit search paths will be discoverd by ValidateSecionModules().
bool discover_implicit_search_paths = false;
if (DeserializeAllCompilerFlags(swift_ast_sp->GetCompilerInvocation(),
module, discover_implicit_search_paths,
m_description, errs, got_serialized_options,
found_swift_modules)) {
// Validation errors are not fatal for the context.
swift_ast_sp->m_module_import_warnings.push_back(std::string(error));
}
Expand Down Expand Up @@ -1811,9 +1858,9 @@ ProcessModule(ModuleSP module_sp, std::string m_description,
llvm::SmallString<0> error;
llvm::raw_svector_ostream errs(error);
swift::CompilerInvocation invocation;
if (DeserializeAllCompilerFlags(invocation, *module_sp, m_description, errs,
got_serialized_options,
found_swift_modules)) {
if (DeserializeAllCompilerFlags(
invocation, *module_sp, discover_implicit_search_paths, m_description,
errs, got_serialized_options, found_swift_modules)) {
// TODO: After removing DeserializeAllCompilerFlags from
// CreateInstance(per-Module), errs will need to be
// collected here and surfaced.
Expand All @@ -1823,53 +1870,14 @@ ProcessModule(ModuleSP module_sp, std::string m_description,
module_search_paths.insert(module_search_paths.end(),
opts.getImportSearchPaths().begin(),
opts.getImportSearchPaths().end());
for (auto path:opts.getFrameworkSearchPaths())
framework_search_paths.push_back({path.Path, path.IsSystem});
auto &clang_opts = invocation.getClangImporterOptions().ExtraArgs;
for (const std::string &arg : clang_opts) {
extra_clang_args.push_back(arg);
LOG_VERBOSE_PRINTF(GetLog(LLDBLog::Types), "adding Clang argument \"%s\".",
arg.c_str());
}
// FIXME: Unfortunately this:
//
// for (const auto &fwsp : opts.getFrameworkSearchPaths())
// framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
//
// is insufficient, as ClangImporter can discover more framework
// search paths on the fly. Once a better solution is found,
// warmup_contexts can be retired (again).
{
SymbolFile *sym_file = module_sp->GetSymbolFile();
if (!sym_file)
return;
Status sym_file_error;
auto type_system_or_err =
sym_file->GetTypeSystemForLanguage(lldb::eLanguageTypeSwift);
if (!type_system_or_err) {
llvm::consumeError(type_system_or_err.takeError());
return;
}
auto ts =
llvm::dyn_cast_or_null<TypeSystemSwift>(type_system_or_err->get());
if (!ts)
return;

SwiftASTContext *ast_context = ts->GetSwiftASTContext();
if (!ast_context || ast_context->HasErrors())
return;

if (discover_implicit_search_paths) {
const auto &opts = ast_context->GetSearchPathOptions();
for (const auto &isp : opts.getImportSearchPaths())
module_search_paths.push_back(isp);
}

if (use_all_compiler_flags ||
target.GetExecutableModulePointer() == module_sp.get()) {
const auto &opts = ast_context->GetSearchPathOptions();
for (const auto &fwsp : opts.getFrameworkSearchPaths())
framework_search_paths.push_back({fwsp.Path, fwsp.IsSystem});
}
}
}

lldb::TypeSystemSP SwiftASTContext::CreateInstance(
Expand Down Expand Up @@ -1930,26 +1938,7 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
handled_sdk_path = true;
}

auto warmup_astcontexts = [&]() {
if (target.GetSwiftCreateModuleContextsInParallel()) {
// The first call to GetTypeSystemForLanguage() on a module will
// trigger the import (and thus most likely the rebuild) of all
// the Clang modules that were imported in this module. This can
// be a lot of work (potentially ten seconds per module), but it
// can be performed in parallel.
llvm::ThreadPool pool(llvm::hardware_concurrency());
for (size_t mi = 0; mi != num_images; ++mi) {
auto module_sp = target.GetImages().GetModuleAtIndex(mi);
pool.async([=] {
GetModuleSwiftASTContext(*module_sp);
});
}
pool.wait();
}
};

if (!handled_sdk_path) {
warmup_astcontexts();
for (size_t mi = 0; mi != num_images; ++mi) {
ModuleSP module_sp = target.GetImages().GetModuleAtIndex(mi);
if (!HasSwiftModules(*module_sp))
Expand Down Expand Up @@ -2066,8 +2055,6 @@ lldb::TypeSystemSP SwiftASTContext::CreateInstance(
FileSpec(resource_dir), triple);
const bool discover_implicit_search_paths =
target.GetSwiftDiscoverImplicitSearchPaths();
if (discover_implicit_search_paths)
warmup_astcontexts();

const bool use_all_compiler_flags =
!got_serialized_options || target.GetUseAllCompilerFlags();
Expand Down
12 changes: 0 additions & 12 deletions lldb/source/Target/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4495,18 +4495,6 @@ void TargetProperties::SetInjectLocalVariables(ExecutionContext *exe_ctx,
true);
}

bool TargetProperties::GetSwiftCreateModuleContextsInParallel() const {
const Property *exp_property = m_collection_sp->GetPropertyAtIndex(
nullptr, false, ePropertyExperimental);
OptionValueProperties *exp_values =
exp_property->GetValue()->GetAsProperties();
if (exp_values)
return exp_values->GetPropertyAtIndexAsBoolean(
nullptr, ePropertySwiftCreateModuleContextsInParallel, true);
else
return true;
}

bool TargetProperties::GetSwiftReadMetadataFromFileCache() const {
const Property *exp_property = m_collection_sp->GetPropertyAtIndex(
nullptr, false, ePropertyExperimental);
Expand Down
3 changes: 0 additions & 3 deletions lldb/source/Target/TargetProperties.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ let Definition = "target_experimental" in {
def InjectLocalVars : Property<"inject-local-vars", "Boolean">,
Global, DefaultTrue,
Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
def SwiftCreateModuleContextsInParallel : Property<"swift-create-module-contexts-in-parallel", "Boolean">,
DefaultTrue,
Desc<"Create the per-module Swift AST contexts in parallel.">;
def SwiftReadMetadataFromFileCache: Property<"swift-read-metadata-from-file-cache", "Boolean">,
DefaultTrue,
Desc<"Read Swift reflection metadata from the file cache instead of the process when possible">;
Expand Down