Skip to content

Commit 0ea036b

Browse files
authored
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 3ed13ea commit 0ea036b

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
@@ -717,7 +717,8 @@ static inline void ignoreError(T &err) JL_NOTSAFEPOINT
717717
#endif
718718
}
719719

720-
static void get_function_name_and_base(llvm::object::SectionRef Section, size_t pointer, int64_t slide, bool inimage,
720+
static void get_function_name_and_base(llvm::object::SectionRef Section, std::map<uintptr_t, StringRef, std::greater<size_t>> *symbolmap,
721+
size_t pointer, int64_t slide, bool inimage,
721722
void **saddr, char **name, bool untrusted_dladdr) JL_NOTSAFEPOINT
722723
{
723724
bool needs_saddr = saddr && (!*saddr || untrusted_dladdr);
@@ -743,59 +744,73 @@ static void get_function_name_and_base(llvm::object::SectionRef Section, size_t
743744
#endif
744745
}
745746
if (Section.getObject() && (needs_saddr || needs_name)) {
746-
size_t distance = (size_t)-1;
747-
object::SymbolRef sym_found;
748-
for (auto sym : Section.getObject()->symbols()) {
749-
if (!Section.containsSymbol(sym))
750-
continue;
751-
auto addr = sym.getAddress();
752-
if (!addr)
753-
continue;
754-
size_t symptr = addr.get();
755-
if (symptr > pointer + slide)
756-
continue;
757-
size_t new_dist = pointer + slide - symptr;
758-
if (new_dist > distance)
759-
continue;
760-
distance = new_dist;
761-
sym_found = sym;
762-
}
763-
if (distance != (size_t)-1) {
764-
if (needs_saddr) {
765-
uintptr_t addr = cantFail(sym_found.getAddress());
766-
*saddr = (void*)(addr - slide);
767-
needs_saddr = false;
747+
uintptr_t addr = 0;
748+
StringRef nameref{};
749+
{
750+
std::shared_lock<std::shared_mutex> read_lock(getJITDebugRegistry().symbol_mutex);
751+
if (symbolmap->empty()) {
752+
read_lock.unlock();
753+
{
754+
// symbol map hasn't been generated yet, so fill it in now
755+
std::unique_lock<std::shared_mutex> write_lock(getJITDebugRegistry().symbol_mutex);
756+
if (symbolmap->empty()) {
757+
for (auto sym : Section.getObject()->symbols()) {
758+
if (!Section.containsSymbol(sym))
759+
continue;
760+
761+
auto maybe_addr = sym.getAddress();
762+
if (!maybe_addr)
763+
continue;
764+
size_t addr = maybe_addr.get();
765+
766+
auto maybe_nameref = sym.getName();
767+
StringRef nameref{};
768+
if (maybe_nameref)
769+
nameref = maybe_nameref.get();
770+
771+
symbolmap->emplace(addr, nameref);
772+
}
773+
}
774+
}
775+
read_lock.lock();
776+
}
777+
auto fit = symbolmap->lower_bound(pointer + slide);
778+
if (fit != symbolmap->end()) {
779+
addr = fit->first;
780+
nameref = fit->second;
768781
}
769-
if (needs_name) {
770-
if (auto name_or_err = sym_found.getName()) {
771-
auto nameref = name_or_err.get();
772-
const char globalPrefix = // == DataLayout::getGlobalPrefix
782+
}
783+
std::string namerefstr = nameref.str();
784+
if (needs_saddr && addr != 0) {
785+
*saddr = (void*)(addr - slide);
786+
needs_saddr = false;
787+
}
788+
if (needs_name && !nameref.empty()) {
789+
const char globalPrefix = // == DataLayout::getGlobalPrefix
773790
#if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
774-
'_';
791+
'_';
775792
#elif defined(_OS_DARWIN_)
776-
'_';
793+
'_';
777794
#else
778-
'\0';
795+
'\0';
779796
#endif
780-
if (globalPrefix) {
781-
if (nameref[0] == globalPrefix)
782-
nameref = nameref.drop_front();
797+
if (globalPrefix) {
798+
if (nameref[0] == globalPrefix)
799+
nameref = nameref.drop_front();
783800
#if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
784-
else if (nameref[0] == '@') // X86_VectorCall
785-
nameref = nameref.drop_front();
801+
else if (nameref[0] == '@') // X86_VectorCall
802+
nameref = nameref.drop_front();
786803
#endif
787-
// else VectorCall, Assembly, Internal, etc.
788-
}
804+
// else VectorCall, Assembly, Internal, etc.
805+
}
789806
#if defined(_OS_WINDOWS_) && !defined(_CPU_X86_64_)
790-
nameref = nameref.split('@').first;
807+
nameref = nameref.split('@').first;
791808
#endif
792-
size_t len = nameref.size();
793-
*name = (char*)realloc_s(*name, len + 1);
794-
memcpy(*name, nameref.data(), len);
795-
(*name)[len] = 0;
796-
needs_name = false;
797-
}
798-
}
809+
size_t len = nameref.size();
810+
*name = (char*)realloc_s(*name, len + 1);
811+
memcpy(*name, nameref.data(), len);
812+
(*name)[len] = 0;
813+
needs_name = false;
799814
}
800815
}
801816
#ifdef _OS_WINDOWS_
@@ -819,7 +834,7 @@ static void get_function_name_and_base(llvm::object::SectionRef Section, size_t
819834
#endif
820835
}
821836

822-
static objfileentry_t find_object_file(uint64_t fbase, StringRef fname) JL_NOTSAFEPOINT
837+
static jl_object_file_entry_t find_object_file(uint64_t fbase, StringRef fname) JL_NOTSAFEPOINT
823838
{
824839
int isdarwin = 0, islinux = 0, iswindows = 0;
825840
#if defined(_OS_DARWIN_)
@@ -832,7 +847,7 @@ static objfileentry_t find_object_file(uint64_t fbase, StringRef fname) JL_NOTSA
832847
(void)iswindows;
833848

834849
// GOAL: Read debuginfo from file
835-
objfileentry_t entry{nullptr, nullptr, 0};
850+
jl_object_file_entry_t entry{nullptr, nullptr, 0, nullptr};
836851
auto success = getJITDebugRegistry().get_objfile_map()->emplace(fbase, entry);
837852
if (!success.second)
838853
// Return cached value
@@ -1009,7 +1024,8 @@ static objfileentry_t find_object_file(uint64_t fbase, StringRef fname) JL_NOTSA
10091024
auto binary = errorobj->takeBinary();
10101025
binary.first.release();
10111026
binary.second.release();
1012-
entry = {debugobj, context, slide};
1027+
1028+
entry = {debugobj, context, slide, new std::map<uintptr_t, StringRef, std::greater<size_t>>()};
10131029
// update cache
10141030
(*getJITDebugRegistry().get_objfile_map())[fbase] = entry;
10151031
}
@@ -1139,15 +1155,15 @@ bool jl_dylib_DI_for_fptr(size_t pointer, object::SectionRef *Section, int64_t *
11391155
jl_copy_str(filename, dlinfo.dli_fname);
11401156
fname = dlinfo.dli_fname;
11411157
#endif // ifdef _OS_WINDOWS_
1142-
auto entry = find_object_file(fbase, fname);
1158+
jl_object_file_entry_t entry = find_object_file(fbase, fname);
11431159
*slide = entry.slide;
11441160
*context = entry.ctx;
11451161
if (entry.obj)
11461162
*Section = getModuleSectionForAddress(entry.obj, pointer + entry.slide);
11471163
// Assume we only need base address for sysimg for now
11481164
if (!inimage || 0 == image_info.fptrs.nptrs)
11491165
saddr = nullptr;
1150-
get_function_name_and_base(*Section, pointer, entry.slide, inimage, saddr, name, untrusted_dladdr);
1166+
get_function_name_and_base(*Section, entry.symbolmap, pointer, entry.slide, inimage, saddr, name, untrusted_dladdr);
11511167
return true;
11521168
}
11531169

0 commit comments

Comments
 (0)