Some minor improvements on the symtab parsing code

[tberghammer]

Summary

Some minor improvements on the symtab parsing code

• Remove an unnecessary re-computation on arch spec from the ELF file
• Use a local cache to optimize name based section lookup in symtab parsing
• Optimize C++ method basename validation with replacing a regex with hand written code

These modifications reduce the time required to parse the symtab from
large applications by ~25% (tested with LLDB as inferior)

source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

while (pos > 0 && depth > 0)
         else if (s[pos] == right_char)
         {
             ++depth;
         }
     }
     return false;
 }
 
+static bool
+IsValidBasename(const llvm::StringRef& basename)
+{
+    // Check that the basename matches with the following regular expression or is an operator name:
+    // "^~?([A-Za-z_][A-Za-z_0-9]*)(<.*>)?$"
+    // We are using a hand written implementation because it is significantly more efficient then
+    // using the general purpose regular expression library.
+    size_t idx = 0;
+    if (basename.size() > 0 && basename[0] == '~')
+        idx = 1;
+
+    if (basename.size() <= idx)
+        return false; // Empty string or "~"
+
+    if (!std::isalpha(basename[idx]) && basename[idx] != '_')
+        return false; // First charater (after removing the possible '~'') isn't in [A-Za-z_]
+
+    // Read all characters matching [A-Za-z_0-9]
+    ++idx;
+    while (idx < basename.size())
+    {
+        if (!std::isalnum(basename[idx]) && basename[idx] != '_')
+            break;
+        ++idx;
+    }
+
+    // We processed all characters. It is a vaild basename.
+    if (idx == basename.size())
+        return true;
+
+    // Check for basename with template arguments
+    // TODO: Improve the quality of the validation with validating the template arguments
+    if (basename[idx] == '<' && basename.back() == '>')
+        return true;
+
+    // Check if the basename is a vaild C++ operator name
+    if (!basename.startswith("operator"))
+        return false;
+
+    static RegularExpression g_operator_regex("^(operator)( ?)([A-Za-z_][A-Za-z_0-9]*|\\(\\)|\\[\\]|[\\^<>=!\\/*+-]+)(<.*>)?(\\[\\])?$");
+    std::string basename_str(basename.str());
+    return g_operator_regex.Execute(basename_str.c_str(), nullptr);
+}
 
 void
 CPlusPlusLanguage::MethodName::Parse()
 {
     if (!m_parsed && m_full)
     {
 //        ConstString mangled;
 //        m_full.GetMangledCounterpart(mangled);
//            printf ("   mangled = '%s'\n", mangled.GetCString());
                 }
                 m_type = eTypeUnknownMethod;
             }
             else
             {
                 m_parse_error = true;
                 return;
             }
-        
-//            if (!m_context.empty())
-//                printf ("   context = '%s'\n", m_context.str().c_str());
-//            if (m_basename)
-//                printf ("  basename = '%s'\n", m_basename.GetCString());
-//            if (!m_arguments.empty())
-//                printf (" arguments = '%s'\n", m_arguments.str().c_str());
-//            if (!m_qualifiers.empty())
-//                printf ("qualifiers = '%s'\n", m_qualifiers.str().c_str());
-
-            // Make sure we have a valid C++ basename with optional template args
-            static RegularExpression g_identifier_regex("^~?([A-Za-z_][A-Za-z_0-9]*)(<.*>)?$");
-            std::string basename_str(m_basename.str());
-            bool basename_is_valid = g_identifier_regex.Execute (basename_str.c_str(), NULL);
-            if (!basename_is_valid)
-            {
-                // Check for C++ operators
-                if (m_basename.startswith("operator"))
-                {
-                    static RegularExpression g_operator_regex("^(operator)( ?)([A-Za-z_][A-Za-z_0-9]*|\\(\\)|\\[\\]|[\\^<>=!\\/*+-]+)(<.*>)?(\\[\\])?$");
-                    basename_is_valid = g_operator_regex.Execute(basename_str.c_str(), NULL);
-                }
-            }
-            if (!basename_is_valid)
+            if (!IsValidBasename(m_basename))
             {
                 // The C++ basename doesn't match our regular expressions so this can't
                 // be a valid C++ method, clear everything out and indicate an error
                 m_context = llvm::StringRef();
                 m_basename = llvm::StringRef();
                 m_arguments = llvm::StringRef();
                 m_qualifiers = llvm::StringRef();
                 m_parse_error = true;
             }
         }
         else
         {
             m_parse_error = true;
-//            printf ("error: didn't find matching parens for arguments\n");
         }
     }
 }
 
 llvm::StringRef
 CPlusPlusLanguage::MethodName::GetBasename ()
 {
     if (!m_parsed)

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

 //===-- ObjectFileELF.cpp ------------------------------------- -*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ObjectFileELF.h"
 
 #include <cassert>
 #include <algorithm>
+#include <unordered_map>
 
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataBuffer.h"
 #include "lldb/Core/Error.h"
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Core/Log.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
ObjectFileELF::ParseSymbols (Symtab *symtab,
     // slow unwinding speed because the instruction emulation based unwind plans try to emulate all
     // instructions in these symbols. Don't add these symbols to the symbol list as they have no
     // use for the debugger and they are causing a lot of trouble.
     // Filtering can't be restricted to Android because this special object file don't contain the
     // note section specifying the environment to Android but the custom extension and file name
     // makes it highly unlikely that this will collide with anything else.
     bool skip_oatdata_oatexec = m_file.GetFilename() == ConstString("system@[e-mail]");
 
+    ArchSpec arch;
+    GetArchitecture(arch);
+
+    // Local cache to avoid doing a FindSectionByName for each symbol. The "const char*" key must
+    // came from a ConstString object so they can be compared by pointer
+    std::unordered_map<const char*, lldb::SectionSP> section_name_to_section;
+
     unsigned i;
     for (i = 0; i < num_symbols; ++i)
     {
         if (symbol.Parse(symtab_data, &offset) == false)
             break;
         
         const char *symbol_name = strtab_data.PeekCStr(symbol.st_name);
 
for (i = 0; i < num_symbols; ++i)
                     symbol_type = eSymbolTypeData;
                 }
             }
         }
 
         int64_t symbol_value_offset = 0;
         uint32_t additional_flags = 0;
 
-        ArchSpec arch;
-        if (GetArchitecture(arch))
+        if (arch.IsValid())
         {
             if (arch.GetMachine() == llvm::Triple::arm)
             {
                 if (symbol.getBinding() == STB_LOCAL)
                 {
                     char mapping_symbol = FindArmAarch64MappingSymbol(symbol_name);
                     if (symbol_type == eSymbolTypeCode)
                     {
for (i = 0; i < num_symbols; ++i)
         {
             ModuleSP module_sp(GetModule());
             if (module_sp)
             {
                 SectionList *module_section_list = module_sp->GetSectionList();
                 if (module_section_list && module_section_list != section_list)
                 {
                     const ConstString &sect_name = symbol_section_sp->GetName();
-                    lldb::SectionSP section_sp (module_section_list->FindSectionByName (sect_name));
-                    if (section_sp && section_sp->GetFileSize())
-                    {
-                        symbol_section_sp = section_sp;
-                    }
+                    auto section_it = section_name_to_section.find(sect_name.GetCString());
+                    if (section_it == section_name_to_section.end())
+                        section_it = section_name_to_section.emplace(
+                            sect_name.GetCString(),
+                            module_section_list->FindSectionByName (sect_name)).first;
+                    if (section_it->second && section_it->second->GetFileSize())
+                        symbol_section_sp = section_it->second;
                 }
             }
         }
 
         bool is_global = symbol.getBinding() == STB_GLOBAL;
         uint32_t flags = symbol.st_other << 8 | symbol.st_info | additional_flags;
         bool is_mangled = symbol_name ? (symbol_name[0] == '_' && symbol_name[1] == 'Z') : false;