Skip to content

Commit 820ab4a

Browse files
committed
SwiftDWARFImporterDelegate: Cache type lookups to avoid searching the world.
When a type is being looked up on behalf of a per-module context, it is often looked up again immediately afterwards in the scratch context for the purpose of dynamic type resolution. This time hoewever we search all lldb::Modules for the type since the scratch context doesn't know where the query is coming from. This patch adds caching to avoid this situation at the cost of potentially missing scratch-context-only lookups. rdar://problem/56582727
1 parent 0cf3cc9 commit 820ab4a

File tree

10 files changed

+242
-32
lines changed

10 files changed

+242
-32
lines changed

lldb/include/lldb/Symbol/SwiftASTContext.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ class SwiftASTContext : public TypeSystem {
125125

126126
~SwiftASTContext();
127127

128+
const std::string &GetDescription() const;
129+
128130
// PluginInterface functions
129131
ConstString GetPluginName() override;
130132

@@ -313,6 +315,7 @@ class SwiftASTContext : public TypeSystem {
313315
CompilerType ImportType(CompilerType &type, Status &error);
314316

315317
swift::ClangImporter *GetClangImporter();
318+
swift::DWARFImporterDelegate *GetDWARFImporterDelegate();
316319

317320
struct TupleElement {
318321
ConstString element_name;

lldb/packages/Python/lldbsuite/test/lang/swift/dwarfimporter/Objective-C/TestSwiftDWARFImporterObjC.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ def build(self):
3939
@swiftTest
4040
def test_dwarf_importer(self):
4141
self.runCmd("settings set symbols.use-swift-dwarfimporter true")
42+
4243
self.build()
4344
target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
4445
self, 'break here', lldb.SBFileSpec('main.swift'))
@@ -58,5 +59,5 @@ def test_dwarf_importer(self):
5859
# This is a Clang type, since Clang doesn't generate DWARF for protocols.
5960
self.expect("target var -d no-dyn proto", substrs=["(id)", "proto"])
6061
# This is a Swift type.
61-
self.expect("target var -d run proto", substrs=["(ProtoImpl?)", "proto"])
62+
self.expect("target var -d run proto", substrs=["(ProtoImpl)", "proto"])
6263
self.expect("target var -O proto", substrs=["<ProtoImpl"])
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
module ObjCModule {
2+
header "objc-header.h"
3+
export *
4+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/* -*- ObjC -*- */
2+
@import Foundation;
3+
4+
@interface ObjCClass : NSObject
5+
- (instancetype)init;
6+
- (NSString *)getString;
7+
- (id)getMangled;
8+
- (id)getRawname;
9+
@end
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import Foundation
2+
3+
/// This is an object exposed to Objective-C as "_Tt7Library10SwiftClassC".
4+
@objc public class MangledSwiftClass : NSObject {
5+
@objc public func getString() -> NSString { return "Hello from Swift!" }
6+
}
7+
8+
/// This is an object exposed to Objective-C as "RawNameSwiftClass".
9+
@objc(RawNameSwiftClass) public class RawNameSwiftClass : NSObject {
10+
@objc public func getString() -> NSString { return "Hello from Swift!" }
11+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
LEVEL = ../../../../make
2+
3+
MAKE_GMODULES := YES
4+
SWIFT_OBJC_INTEROP = 1
5+
SWIFT_SOURCES := main.swift
6+
OBJC_SOURCES := objc.m
7+
CFLAGS_EXTRAS = -I$(BUILDDIR)/include
8+
SWIFTFLAGS_EXTRAS = -I$(BUILDDIR)/include
9+
LD_EXTRAS := libLibrary.dylib
10+
11+
include $(LEVEL)/Makefile.rules
12+
13+
objc.o: libLibrary.dylib
14+
15+
libLibrary.dylib: Library.swift
16+
$(MAKE) -f $(MAKEFILE_RULES) \
17+
DYLIB_SWIFT_SOURCES=Library.swift \
18+
DYLIB_NAME=Library \
19+
SWIFTFLAGS_EXTRAS="-emit-objc-header-path $(BUILDDIR)/include/Library-Swift.h"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import lldb
2+
from lldbsuite.test.decorators import *
3+
import lldbsuite.test.lldbtest as lldbtest
4+
import lldbsuite.test.lldbutil as lldbutil
5+
import os
6+
import unittest2
7+
8+
9+
class TestSwiftDWARFImporter_Swift(lldbtest.TestBase):
10+
11+
mydir = lldbtest.TestBase.compute_mydir(__file__)
12+
13+
def build(self):
14+
include = self.getBuildArtifact('include')
15+
inputs = self.getSourcePath(os.path.join('Inputs', 'Modules'))
16+
lldbutil.mkdir_p(include)
17+
import shutil
18+
for f in ['module.modulemap', 'objc-header.h']:
19+
shutil.copyfile(os.path.join(inputs, f), os.path.join(include, f))
20+
21+
super(TestSwiftDWARFImporter_Swift, self).build()
22+
23+
# Remove the header files to thwart ClangImporter.
24+
self.assertTrue(os.path.isdir(include))
25+
shutil.rmtree(include)
26+
27+
@skipUnlessDarwin
28+
@swiftTest
29+
def test(self):
30+
self.runCmd("settings set symbols.use-swift-dwarfimporter true")
31+
self.build()
32+
target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
33+
self, 'break here', lldb.SBFileSpec('main.swift'))
34+
35+
log = self.getBuildArtifact("types.log")
36+
self.runCmd('log enable lldb types -f "%s"' % log)
37+
38+
self.expect("target var -d run myobj", substrs=["(ObjCClass)"])
39+
40+
found = 0
41+
response = 0
42+
logfile = open(log, "r")
43+
for line in logfile:
44+
if 'SwiftDWARFImporterDelegate::lookupValue("ObjCClass")' in line:
45+
found += 1
46+
elif found == 1 and response == 0 and 'SwiftDWARFImporterDelegate' in line:
47+
self.assertTrue('from debug info' in line, line)
48+
response += 1
49+
elif found == 2 and response == 1 and 'SwiftDWARFImporterDelegate' in line:
50+
self.assertTrue('found in cache' in line, line)
51+
response += 1
52+
self.assertEqual(found, 2)
53+
self.assertEqual(response, 2)
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import ObjCModule
2+
3+
func use<T>(_ t : T) {}
4+
5+
guard let obj = ObjCClass() else { exit(1) }
6+
let myobj = obj
7+
let mangled = obj.getMangled()!
8+
let rawname = obj.getRawname()!
9+
let s = obj.getString()
10+
use(s) // break here
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
@import ObjCModule;
2+
#include "Library-Swift.h"
3+
4+
@implementation ObjCClass {
5+
id mangled_swift_obj;
6+
id rawname_swift_obj;
7+
}
8+
9+
- (instancetype)init {
10+
self = [super init];
11+
if (self) {
12+
mangled_swift_obj = [MangledSwiftClass new];
13+
rawname_swift_obj = [RawNameSwiftClass new];
14+
}
15+
return self;
16+
}
17+
18+
- (id)getMangled {
19+
return mangled_swift_obj;
20+
}
21+
22+
- (id)getRawname {
23+
return rawname_swift_obj;
24+
}
25+
26+
- (NSString *)getString {
27+
return [mangled_swift_obj getString];
28+
}
29+
30+
@end
31+

lldb/source/Symbol/SwiftASTContext.cpp

Lines changed: 100 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,8 @@ SwiftASTContext::~SwiftASTContext() {
920920
}
921921
}
922922

923+
const std::string &SwiftASTContext::GetDescription() const { return m_description; }
924+
923925
ConstString SwiftASTContext::GetPluginNameStatic() {
924926
return ConstString("swift");
925927
}
@@ -3103,6 +3105,11 @@ class StoringDiagnosticConsumer : public swift::DiagnosticConsumer {
31033105
/// Clang AST to ClangImporter to import the type into Swift.
31043106
class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
31053107
SwiftASTContext &m_swift_ast_ctx;
3108+
using ModuleAndName = std::pair<const char *, const char *>;
3109+
/// Caches successful lookups for the scratch context.
3110+
llvm::DenseMap<ModuleAndName, llvm::SmallVector<clang::QualType, 1>>
3111+
m_decl_cache;
3112+
std::string m_description;
31063113

31073114
/// Used to filter out types with mismatching kinds.
31083115
bool HasTypeKind(TypeSP clang_type_sp, swift::ClangTypeKind kind) {
@@ -3170,9 +3177,42 @@ class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
31703177
}
31713178
}
31723179

3180+
/// Import \p qual_type from one clang ASTContext to another and
3181+
/// add it to \p results if successful.
3182+
void importType(clang::QualType qual_type, clang::ASTContext &from_ctx,
3183+
clang::ASTContext &to_ctx,
3184+
llvm::Optional<swift::ClangTypeKind> kind,
3185+
llvm::SmallVectorImpl<clang::Decl *> &results) {
3186+
clang::FileSystemOptions file_system_options;
3187+
clang::FileManager file_manager(
3188+
file_system_options, FileSystem::Instance().GetVirtualFileSystem());
3189+
clang::ASTImporter importer(to_ctx, file_manager, from_ctx, file_manager,
3190+
false);
3191+
llvm::Expected<clang::QualType> clang_type(importer.Import(qual_type));
3192+
if (!clang_type) {
3193+
llvm::consumeError(clang_type.takeError());
3194+
return;
3195+
}
3196+
3197+
// Retrieve the imported type's Decl.
3198+
if (kind) {
3199+
if (clang::Decl *clang_decl = GetDeclForTypeAndKind(*clang_type, *kind))
3200+
results.push_back(clang_decl);
3201+
} else {
3202+
swift::ClangTypeKind kinds[] = {
3203+
swift::ClangTypeKind::Typedef, // =swift::ClangTypeKind::ObjCClass,
3204+
swift::ClangTypeKind::Tag, swift::ClangTypeKind::ObjCProtocol};
3205+
for (auto kind : kinds)
3206+
if (clang::Decl *clang_decl = GetDeclForTypeAndKind(*clang_type, kind))
3207+
results.push_back(clang_decl);
3208+
}
3209+
}
3210+
31733211
public:
31743212
SwiftDWARFImporterDelegate(SwiftASTContext &swift_ast_ctx)
3175-
: m_swift_ast_ctx(swift_ast_ctx) {}
3213+
: m_swift_ast_ctx(swift_ast_ctx),
3214+
m_description(swift_ast_ctx.GetDescription() +
3215+
"::SwiftDWARFImporterDelegate") {}
31763216

31773217
/// Look up a clang::Decl by name.
31783218
///
@@ -3231,6 +3271,8 @@ class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
32313271
void lookupValue(StringRef name, llvm::Optional<swift::ClangTypeKind> kind,
32323272
StringRef inModule,
32333273
llvm::SmallVectorImpl<clang::Decl *> &results) override {
3274+
LOG_PRINTF(LIBLLDB_LOG_TYPES, "(\"%s\")", name.str().c_str());
3275+
32343276
// We will not find any Swift types in the Clang compile units.
32353277
if (SwiftLanguageRuntime::IsSwiftMangledName(name.str().c_str()))
32363278
return;
@@ -3241,15 +3283,16 @@ class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
32413283

32423284
// Find the type in the debug info.
32433285
TypeMap clang_types;
3286+
ConstString name_cs(name);
3287+
ConstString module_cs(inModule);
32443288

32453289
llvm::SmallVector<CompilerContext, 3> decl_context;
32463290
// Perform a lookup in a specific module, if requested.
32473291
if (!inModule.empty())
3248-
decl_context.push_back(
3249-
{CompilerContextKind::Module, ConstString(inModule)});
3292+
decl_context.push_back({CompilerContextKind::Module, module_cs});
32503293
// Swift doesn't keep track of submodules.
32513294
decl_context.push_back({CompilerContextKind::AnyModule, ConstString()});
3252-
decl_context.push_back({GetCompilerContextKind(kind), ConstString(name)});
3295+
decl_context.push_back({GetCompilerContextKind(kind), name_cs});
32533296
auto search = [&](Module &module) {
32543297
return module.FindTypes(decl_context,
32553298
ClangASTContext::GetSupportedLanguagesForTypes(),
@@ -3258,16 +3301,46 @@ class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
32583301
if (Module *module = m_swift_ast_ctx.GetModule())
32593302
search(*module);
32603303
else if (TargetSP target_sp = m_swift_ast_ctx.GetTarget().lock()) {
3261-
// In a scratch context, search everywhere.
3304+
// In a scratch context, check the module's DWARFImporterDelegates first.
3305+
//
3306+
// It's a common pattern that a type is revisited immediately
3307+
// after looking it up in a per-module context in the scratch
3308+
// context for dynamic type resolution.
32623309
auto images = target_sp->GetImages();
3263-
for (size_t i = 0; i != images.GetSize(); ++i)
3264-
if (search(*images.GetModuleAtIndex(i)))
3265-
break;
3310+
for (size_t i = 0; i != images.GetSize(); ++i) {
3311+
auto module_sp = images.GetModuleAtIndex(i);
3312+
auto ts = module_sp->GetTypeSystemForLanguage(lldb::eLanguageTypeSwift);
3313+
if (!ts) {
3314+
llvm::consumeError(ts.takeError());
3315+
continue;
3316+
}
3317+
auto *swift_ast_ctx = static_cast<SwiftASTContext *>(&*ts);
3318+
auto *dwarf_imp = static_cast<SwiftDWARFImporterDelegate *>(
3319+
swift_ast_ctx->GetDWARFImporterDelegate());
3320+
if (!dwarf_imp)
3321+
continue;
3322+
auto it = dwarf_imp->m_decl_cache.find(
3323+
{module_cs.GetCString(), name_cs.GetCString()});
3324+
if (it == dwarf_imp->m_decl_cache.end())
3325+
continue;
3326+
3327+
auto *from_clang_importer = swift_ast_ctx->GetClangImporter();
3328+
if (!from_clang_importer)
3329+
continue;
3330+
auto &from_ctx = from_clang_importer->getClangASTContext();
3331+
auto &to_ctx = clang_importer->getClangASTContext();
3332+
for (clang::QualType qual_type : it->second)
3333+
importType(qual_type, from_ctx, to_ctx, kind, results);
3334+
}
3335+
LOG_PRINTF(LIBLLDB_LOG_TYPES, "%d types found in cache.", results.size());
3336+
3337+
// TODO: Otherwise, the correct thing to do is to invoke
3338+
// search() on all modules. In practice, however, this is
3339+
// prohibitively expensive, so we need to do something
3340+
// more targeted.
3341+
return;
32663342
}
32673343

3268-
clang::FileSystemOptions file_system_options;
3269-
clang::FileManager file_manager(
3270-
file_system_options, FileSystem::Instance().GetVirtualFileSystem());
32713344
clang_types.ForEach([&](lldb::TypeSP &clang_type_sp) {
32723345
if (!clang_type_sp)
32733346
return true;
@@ -3290,29 +3363,19 @@ class SwiftDWARFImporterDelegate : public swift::DWARFImporterDelegate {
32903363
clang::ASTContext *from_ctx = type_system->getASTContext();
32913364
if (!from_ctx)
32923365
return true;
3293-
clang::ASTImporter importer(to_ctx, file_manager, *from_ctx, file_manager,
3294-
false);
3295-
llvm::Expected<clang::QualType> clang_type(
3296-
importer.Import(ClangUtil::GetQualType(compiler_type)));
3297-
if (!clang_type) {
3298-
llvm::consumeError(clang_type.takeError());
3299-
return true;
3300-
}
33013366

3302-
// Retrieve the imported type's Decl.
3303-
if (kind) {
3304-
if (clang::Decl *clang_decl = GetDeclForTypeAndKind(*clang_type, *kind))
3305-
results.push_back(clang_decl);
3306-
} else {
3307-
swift::ClangTypeKind kinds[] = {
3308-
swift::ClangTypeKind::Typedef, // =swift::ClangTypeKind::ObjCClass,
3309-
swift::ClangTypeKind::Tag, swift::ClangTypeKind::ObjCProtocol};
3310-
for (auto kind : kinds)
3311-
if (clang::Decl *clang_decl = GetDeclForTypeAndKind(*clang_type, kind))
3312-
results.push_back(clang_decl);
3313-
}
3367+
clang::QualType qual_type = ClangUtil::GetQualType(compiler_type);
3368+
importType(qual_type, *from_ctx, to_ctx, kind, results);
3369+
3370+
// If this is a module context, cache the result for the scratch context.
3371+
if (m_swift_ast_ctx.GetModule())
3372+
m_decl_cache[{module_cs.GetCString(), name_cs.GetCString()}].push_back(
3373+
qual_type);
3374+
33143375
return true;
33153376
});
3377+
3378+
LOG_PRINTF(LIBLLDB_LOG_TYPES, "%d types from debug info.", results.size());
33163379
}
33173380
};
33183381
} // namespace lldb_private
@@ -3492,6 +3555,12 @@ swift::ClangImporter *SwiftASTContext::GetClangImporter() {
34923555
return m_clang_importer;
34933556
}
34943557

3558+
swift::DWARFImporterDelegate *SwiftASTContext::GetDWARFImporterDelegate() {
3559+
VALID_OR_RETURN(nullptr);
3560+
3561+
return m_dwarf_importer_delegate_up.get();
3562+
}
3563+
34953564
bool SwiftASTContext::AddClangArgument(std::string clang_arg, bool unique) {
34963565
if (clang_arg.empty())
34973566
return false;

0 commit comments

Comments
 (0)