From 2038d2c49eabc9cabb6048ee14b22d6565338c37 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Sat, 28 Aug 2021 05:13:42 -0700 Subject: [PATCH] LibELF: Apply some minor optimizations to symbol lookup Optimizations: - Make sure `DT_SYMTAB` is a string view literal, instead of string. - DynamicObject::HashSection::lookup_sysv_symbol should be using raw_name() from symbol comparison to avoid needlessly calling `strlen`, when the StrinView::operator= walks the cstring without calling `strlen` first. - DynamicObject::HashSection::lookup_gnu_symbol shouldn't create a symbol unless we know the hashes match first. In order to test these changes I enabled Undefined behavior sanitizer which creates a huge amount of relocations, and then ran the browser with the help argument 100 times. The browser is a fairly big app with a few different libraries being loaded, so it seemed liked a good target. Command: `time -n 100 br --help` Before: ``` Timing report: ============== Command: br --help Average time: 3897.679931 ms Excluding first: 3901.242431 ms ``` After: ``` Timing report: ============== Command: br --help Average time: 3612.860107 ms Excluding first: 3613.54541 ms ``` --- Userland/Libraries/LibELF/DynamicObject.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Userland/Libraries/LibELF/DynamicObject.cpp b/Userland/Libraries/LibELF/DynamicObject.cpp index 234c9ac97e..a409938400 100644 --- a/Userland/Libraries/LibELF/DynamicObject.cpp +++ b/Userland/Libraries/LibELF/DynamicObject.cpp @@ -204,7 +204,7 @@ DynamicObject::Relocation DynamicObject::RelocationSection::relocation_at_offset DynamicObject::Symbol DynamicObject::symbol(unsigned index) const { - auto symbol_section = Section(*this, m_symbol_table_offset, (m_symbol_count * m_size_of_symbol_table_entry), m_size_of_symbol_table_entry, "DT_SYMTAB"); + auto symbol_section = Section(*this, m_symbol_table_offset, (m_symbol_count * m_size_of_symbol_table_entry), m_size_of_symbol_table_entry, "DT_SYMTAB"sv); auto symbol_entry = (ElfW(Sym)*)symbol_section.address().offset(index * symbol_section.entry_size()).as_ptr(); return Symbol(*this, index, *symbol_entry); } @@ -267,7 +267,7 @@ auto DynamicObject::HashSection::lookup_sysv_symbol(const StringView& name, u32 for (u32 i = buckets[hash_value % num_buckets]; i; i = chains[i]) { auto symbol = m_dynamic.symbol(i); - if (name == symbol.name()) { + if (name == symbol.raw_name()) { dbgln_if(DYNAMIC_LOAD_DEBUG, "Returning SYSV dynamic symbol with index {} for {}: {}", i, symbol.name(), symbol.address().as_ptr()); return symbol; } @@ -308,9 +308,12 @@ auto DynamicObject::HashSection::lookup_gnu_symbol(const StringView& name, u32 h for (hash1 &= ~1;; ++current_sym) { hash2 = *(current_chain++); - auto symbol = m_dynamic.symbol(current_sym); - if ((hash1 == (hash2 & ~1)) && name == symbol.raw_name()) - return symbol; + if (hash1 == (hash2 & ~1)) { + auto symbol = m_dynamic.symbol(current_sym); + if (name == symbol.raw_name()) + return symbol; + } + if (hash2 & 1) break; }