Skip to content

Commit c37ee51

Browse files
committed
debuginfo: Memoize object symbol lookup (#58851)
Supersedes #58355. Resolves #58326. On this PR: ```julia julia> @Btime lgamma(2.0) ┌ Warning: `lgamma(x::Real)` is deprecated, use `(logabsgamma(x))[1]` instead. │ caller = var"##core#283"() at execution.jl:598 └ @ Core ~/.julia/packages/BenchmarkTools/1i1mY/src/execution.jl:598 47.730 μs (105 allocations: 13.24 KiB) ``` On `nightly`: ```julia julia> @Btime lgamma(2.0) ┌ Warning: `lgamma(x::Real)` is deprecated, use `(logabsgamma(x))[1]` instead. │ caller = var"##core#283"() at execution.jl:598 └ @ Core ~/.julia/packages/BenchmarkTools/1i1mY/src/execution.jl:598 26.856 ms (89 allocations: 11.32 KiB) ```
1 parent 2e45e64 commit c37ee51

File tree

2 files changed

+71
-52
lines changed

2 files changed

+71
-52
lines changed

src/debug-registry.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ typedef struct {
1212
const llvm::object::ObjectFile *obj;
1313
llvm::DIContext *ctx;
1414
int64_t slide;
15-
} objfileentry_t;
15+
std::map<uintptr_t, StringRef, std::greater<size_t>> *symbolmap;
16+
} jl_object_file_entry_t;
1617

1718
// Central registry for resolving function addresses to `jl_code_instance_t`s and
1819
// originating `ObjectFile`s (for the DWARF debug info).
@@ -121,7 +122,7 @@ class JITDebugInfoRegistry
121122
using rev_map = std::map<KeyT, ValT, std::greater<KeyT>>;
122123

123124
typedef rev_map<size_t, SectionInfo> objectmap_t;
124-
typedef rev_map<uint64_t, objfileentry_t> objfilemap_t;
125+
typedef rev_map<uint64_t, jl_object_file_entry_t> objfilemap_t;
125126

126127
objectmap_t objectmap{};
127128
rev_map<size_t, std::pair<size_t, jl_code_instance_t *>> cimap{};
@@ -152,4 +153,6 @@ class JITDebugInfoRegistry
152153
void add_image_info(image_info_t info) JL_NOTSAFEPOINT;
153154
bool get_image_info(uint64_t base, image_info_t *info) const JL_NOTSAFEPOINT;
154155
Locked<objfilemap_t>::LockT get_objfile_map() JL_NOTSAFEPOINT;
156+
157+
std::shared_mutex symbol_mutex;
155158
};

src/debuginfo.cpp

Lines changed: 66 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,8 @@ static inline void ignoreError(T &err) JL_NOTSAFEPOINT
706706
#endif
707707
}
708708

709-
static void get_function_name_and_base(llvm::object::SectionRef Section, size_t pointer, int64_t slide, bool inimage,
709+
static void get_function_name_and_base(llvm::object::SectionRef Section, std::map<uintptr_t, StringRef, std::greater<size_t>> *symbolmap,
710+
size_t pointer, int64_t slide, bool inimage,
710711
void **saddr, char **name, bool untrusted_dladdr) JL_NOTSAFEPOINT
711712
{
712713
bool needs_saddr = saddr && (!*saddr || untrusted_dladdr);
@@ -732,59 +733,73 @@ static void get_function_name_and_base(llvm::object::SectionRef Section, size_t
732733
#endif
733734
}
734735
if (Section.getObject() && (needs_saddr || needs_name)) {
735-
size_t distance = (size_t)-1;
736-
object::SymbolRef sym_found;
737-
for (auto sym : Section.getObject()->symbols()) {
738-
if (!Section.containsSymbol(sym))
739-
continue;
740-
auto addr = sym.getAddress();
741-
if (!addr)
742-
continue;
743-
size_t symptr = addr.get();
744-
if (symptr > pointer + slide)
745-
continue;
746-
size_t new_dist = pointer + slide - symptr;
747-
if (new_dist > distance)
748-
continue;
749-
distance = new_dist;
750-
sym_found = sym;
751-
}
752-
if (distance != (size_t)-1) {
753-
if (needs_saddr) {
754-
uintptr_t addr = cantFail(sym_found.getAddress());
755-
*saddr = (void*)(addr - slide);
756-
needs_saddr = false;
736+
uintptr_t addr = 0;
737+
StringRef nameref{};
738+
{
739+
std::shared_lock<std::shared_mutex> read_lock(getJITDebugRegistry().symbol_mutex);
740+
if (symbolmap->empty()) {
741+
read_lock.unlock();
742+
{
743+
// symbol map hasn't been generated yet, so fill it in now
744+
std::unique_lock<std::shared_mutex> write_lock(getJITDebugRegistry().symbol_mutex);
745+
if (symbolmap->empty()) {
746+
for (auto sym : Section.getObject()->symbols()) {
747+
if (!Section.containsSymbol(sym))
748+
continue;
749+
750+
auto maybe_addr = sym.getAddress();
751+
if (!maybe_addr)
752+
continue;
753+
size_t addr = maybe_addr.get();
754+
755+
auto maybe_nameref = sym.getName();
756+
StringRef nameref{};
757+
if (maybe_nameref)
758+
nameref = maybe_nameref.get();
759+
760+
symbolmap->emplace(addr, nameref);
761+
}
762+
}
763+
}
764+
read_lock.lock();
765+
}
766+
auto fit = symbolmap->lower_bound(pointer + slide);
767+
if (fit != symbolmap->end()) {
768+
addr = fit->first;
769+
nameref = fit->second;
757770
}
758-
if (needs_name) {
759-
if (auto name_or_err = sym_found.getName()) {
760-
auto nameref = name_or_err.get();
761-
const char globalPrefix = // == DataLayout::getGlobalPrefix
771+
}
772+
std::string namerefstr = nameref.str();
773+
if (needs_saddr && addr != 0) {
774+
*saddr = (void*)(addr - slide);
775+
needs_saddr = false;
776+
}
777+
if (needs_name && !nameref.empty()) {
778+
const char globalPrefix = // == DataLayout::getGlobalPrefix
762779
#if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
763-
'_';
780+
'_';
764781
#elif defined(_OS_DARWIN_)
765-
'_';
782+
'_';
766783
#else
767-
'\0';
784+
'\0';
768785
#endif
769-
if (globalPrefix) {
770-
if (nameref[0] == globalPrefix)
771-
nameref = nameref.drop_front();
786+
if (globalPrefix) {
787+
if (nameref[0] == globalPrefix)
788+
nameref = nameref.drop_front();
772789
#if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
773-
else if (nameref[0] == '@') // X86_VectorCall
774-
nameref = nameref.drop_front();
790+
else if (nameref[0] == '@') // X86_VectorCall
791+
nameref = nameref.drop_front();
775792
#endif
776-
// else VectorCall, Assembly, Internal, etc.
777-
}
793+
// else VectorCall, Assembly, Internal, etc.
794+
}
778795
#if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
779-
nameref = nameref.split('@').first;
796+
nameref = nameref.split('@').first;
780797
#endif
781-
size_t len = nameref.size();
782-
*name = (char*)realloc_s(*name, len + 1);
783-
memcpy(*name, nameref.data(), len);
784-
(*name)[len] = 0;
785-
needs_name = false;
786-
}
787-
}
798+
size_t len = nameref.size();
799+
*name = (char*)realloc_s(*name, len + 1);
800+
memcpy(*name, nameref.data(), len);
801+
(*name)[len] = 0;
802+
needs_name = false;
788803
}
789804
}
790805
#ifdef _OS_WINDOWS_
@@ -808,7 +823,7 @@ static void get_function_name_and_base(llvm::object::SectionRef Section, size_t
808823
#endif
809824
}
810825

811-
static objfileentry_t find_object_file(uint64_t fbase, StringRef fname) JL_NOTSAFEPOINT
826+
static jl_object_file_entry_t find_object_file(uint64_t fbase, StringRef fname) JL_NOTSAFEPOINT
812827
{
813828
int isdarwin = 0, islinux = 0, iswindows = 0;
814829
#if defined(_OS_DARWIN_)
@@ -821,7 +836,7 @@ static objfileentry_t find_object_file(uint64_t fbase, StringRef fname) JL_NOTSA
821836
(void)iswindows;
822837

823838
// GOAL: Read debuginfo from file
824-
objfileentry_t entry{nullptr, nullptr, 0};
839+
jl_object_file_entry_t entry{nullptr, nullptr, 0, nullptr};
825840
auto success = getJITDebugRegistry().get_objfile_map()->emplace(fbase, entry);
826841
if (!success.second)
827842
// Return cached value
@@ -995,7 +1010,8 @@ static objfileentry_t find_object_file(uint64_t fbase, StringRef fname) JL_NOTSA
9951010
auto binary = errorobj->takeBinary();
9961011
binary.first.release();
9971012
binary.second.release();
998-
entry = {debugobj, context, slide};
1013+
1014+
entry = {debugobj, context, slide, new std::map<uintptr_t, StringRef, std::greater<size_t>>()};
9991015
// update cache
10001016
(*getJITDebugRegistry().get_objfile_map())[fbase] = entry;
10011017
}
@@ -1125,15 +1141,15 @@ bool jl_dylib_DI_for_fptr(size_t pointer, object::SectionRef *Section, int64_t *
11251141
jl_copy_str(filename, dlinfo.dli_fname);
11261142
fname = dlinfo.dli_fname;
11271143
#endif // ifdef _OS_WINDOWS_
1128-
auto entry = find_object_file(fbase, fname);
1144+
jl_object_file_entry_t entry = find_object_file(fbase, fname);
11291145
*slide = entry.slide;
11301146
*context = entry.ctx;
11311147
if (entry.obj)
11321148
*Section = getModuleSectionForAddress(entry.obj, pointer + entry.slide);
11331149
// Assume we only need base address for sysimg for now
11341150
if (!inimage || 0 == image_info.fptrs.nptrs)
11351151
saddr = nullptr;
1136-
get_function_name_and_base(*Section, pointer, entry.slide, inimage, saddr, name, untrusted_dladdr);
1152+
get_function_name_and_base(*Section, entry.symbolmap, pointer, entry.slide, inimage, saddr, name, untrusted_dladdr);
11371153
return true;
11381154
}
11391155

0 commit comments

Comments
 (0)