From 1adf06c9f0a44dc62b63dd38795204b3400f00ed Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Thu, 17 Aug 2023 08:00:08 +0200 Subject: [PATCH] LibELF: Cache consecutive lookups for the same symbol This reduces the startup time of LibWeb by 10%, and eliminates 156'000 of the total 481'000 global symbol lookups during a self-test run. --- Userland/Libraries/LibELF/DynamicLoader.cpp | 25 ++++++++++++++++----- Userland/Libraries/LibELF/DynamicLoader.h | 6 ++++- Userland/Libraries/LibELF/DynamicObject.h | 6 +++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/Userland/Libraries/LibELF/DynamicLoader.cpp b/Userland/Libraries/LibELF/DynamicLoader.cpp index b1a8ce808d..c9604106f5 100644 --- a/Userland/Libraries/LibELF/DynamicLoader.cpp +++ b/Userland/Libraries/LibELF/DynamicLoader.cpp @@ -209,8 +209,9 @@ void DynamicLoader::do_main_relocations() { do_relr_relocations(); + Optional cached_result; m_dynamic_object->relocation_section().for_each_relocation([&](DynamicObject::Relocation const& relocation) { - switch (do_direct_relocation(relocation, ShouldInitializeWeak::No, ShouldCallIfuncResolver::No)) { + switch (do_direct_relocation(relocation, cached_result, ShouldInitializeWeak::No, ShouldCallIfuncResolver::No)) { case RelocationResult::Failed: dbgln("Loader.so: {} unresolved symbol '{}'", m_filepath, relocation.symbol().name()); VERIFY_NOT_REACHED(); @@ -281,8 +282,9 @@ Result, DlErrorMessage> DynamicLoader::load_stage_3 VERIFY(result == RelocationResult::Success); } + Optional cached_result; for (auto const& relocation : m_direct_ifunc_relocations) { - auto result = do_direct_relocation(relocation, ShouldInitializeWeak::No, ShouldCallIfuncResolver::Yes); + auto result = do_direct_relocation(relocation, cached_result, ShouldInitializeWeak::No, ShouldCallIfuncResolver::Yes); VERIFY(result == RelocationResult::Success); } @@ -321,8 +323,9 @@ void DynamicLoader::load_stage_4() void DynamicLoader::do_lazy_relocations() { + Optional cached_result; for (auto const& relocation : m_unresolved_relocations) { - if (auto res = do_direct_relocation(relocation, ShouldInitializeWeak::Yes, ShouldCallIfuncResolver::Yes); res != RelocationResult::Success) { + if (auto res = do_direct_relocation(relocation, cached_result, ShouldInitializeWeak::Yes, ShouldCallIfuncResolver::Yes); res != RelocationResult::Success) { dbgln("Loader.so: {} unresolved symbol '{}'", m_filepath, relocation.symbol().name()); VERIFY_NOT_REACHED(); } @@ -522,7 +525,10 @@ void DynamicLoader::load_program_headers() } } -DynamicLoader::RelocationResult DynamicLoader::do_direct_relocation(DynamicObject::Relocation const& relocation, ShouldInitializeWeak should_initialize_weak, ShouldCallIfuncResolver should_call_ifunc_resolver) +DynamicLoader::RelocationResult DynamicLoader::do_direct_relocation(DynamicObject::Relocation const& relocation, + Optional& cached_result, + ShouldInitializeWeak should_initialize_weak, + ShouldCallIfuncResolver should_call_ifunc_resolver) { FlatPtr* patch_ptr = nullptr; if (is_dynamic()) @@ -534,12 +540,21 @@ DynamicLoader::RelocationResult DynamicLoader::do_direct_relocation(DynamicObjec return VirtualAddress { reinterpret_cast(address.get())() }; }; + auto lookup_symbol = [&](DynamicObject::Symbol const& symbol) { + // The static linker sorts relocations by the referenced symbol. Especially when vtables + // in large inheritance hierarchies are involved, there might be tens of references to + // the same symbol. We can avoid redundant lookups by keeping track of the previous result. + if (!cached_result.has_value() || !cached_result.value().symbol.definitely_equals(symbol)) + cached_result = DynamicLoader::CachedLookupResult { symbol, DynamicLoader::lookup_symbol(symbol) }; + return cached_result.value().result; + }; + struct ResolvedTLSSymbol { DynamicObject const& dynamic_object; FlatPtr value; }; - auto resolve_tls_symbol = [](DynamicObject::Relocation const& relocation) -> Optional { + auto resolve_tls_symbol = [&](DynamicObject::Relocation const& relocation) -> Optional { if (relocation.symbol_index() == 0) return ResolvedTLSSymbol { relocation.dynamic_object(), 0 }; diff --git a/Userland/Libraries/LibELF/DynamicLoader.h b/Userland/Libraries/LibELF/DynamicLoader.h index b944627fa2..755881282a 100644 --- a/Userland/Libraries/LibELF/DynamicLoader.h +++ b/Userland/Libraries/LibELF/DynamicLoader.h @@ -143,7 +143,11 @@ private: ResolveLater = 2, CallIfuncResolver = 3, }; - RelocationResult do_direct_relocation(DynamicObject::Relocation const&, ShouldInitializeWeak, ShouldCallIfuncResolver); + struct CachedLookupResult { + DynamicObject::Symbol symbol; + Optional result; + }; + RelocationResult do_direct_relocation(DynamicObject::Relocation const&, Optional&, ShouldInitializeWeak, ShouldCallIfuncResolver); // Will be called from _fixup_plt_entry, as part of the PLT trampoline static RelocationResult do_plt_relocation(DynamicObject::Relocation const&, ShouldCallIfuncResolver); void do_relr_relocations(); diff --git a/Userland/Libraries/LibELF/DynamicObject.h b/Userland/Libraries/LibELF/DynamicObject.h index e821a61957..6276e771b3 100644 --- a/Userland/Libraries/LibELF/DynamicObject.h +++ b/Userland/Libraries/LibELF/DynamicObject.h @@ -84,6 +84,12 @@ public: } DynamicObject const& object() const { return m_dynamic; } + // This might return false even if the two Symbol objects resolve to the same thing. + bool definitely_equals(Symbol const& other) const + { + return &m_dynamic == &other.m_dynamic && &m_sym == &other.m_sym && m_index == other.m_index; + } + private: DynamicObject const& m_dynamic; const ElfW(Sym) & m_sym;