Merge pull request #236 from adrian-prantl/lldb-perf

Cherry-pick LLDB performance improvements.

Author: adrian-prantl

lldb/include/lldb/Core/Mangled.h

@@ -49,21 +49,7 @@ class Mangled {
   /// Default constructor.
   ///
   /// Initialize with both mangled and demangled names empty.
-  Mangled();
-
-  /// Construct with name.
-  ///
-  /// Constructor with an optional string and a boolean indicating if it is
-  /// the mangled version.
-  ///
-  /// \param[in] name
-  ///     The already const name to copy into this object.
-  ///
-  /// \param[in] is_mangled
-  ///     If \b true then \a name is a mangled name, if \b false then
-  ///     \a name is demangled.
-  Mangled(ConstString name, bool is_mangled);
-  Mangled(llvm::StringRef name, bool is_mangled);
+  Mangled() = default;
 
   /// Construct with name.
   ///
@@ -76,12 +62,6 @@ class Mangled {
 
   explicit Mangled(llvm::StringRef name);
 
-  /// Destructor
-  ///
-  /// Releases its ref counts on the mangled and demangled strings that live
-  /// in the global string pool.
-  ~Mangled();
-
   /// Convert to pointer operator.
   ///
   /// This allows code to check a Mangled object to see if it contains a valid

lldb/include/lldb/Symbol/Function.h

@@ -140,7 +140,7 @@ class InlineFunctionInfo : public FunctionInfo {
   /// \param[in] call_decl_ptr
   ///     Optional calling location declaration information that
   ///     describes from where this inlined function was called.
-  InlineFunctionInfo(const char *name, const char *mangled,
+  InlineFunctionInfo(const char *name, llvm::StringRef mangled,
                      const Declaration *decl_ptr,
                      const Declaration *call_decl_ptr);
 

lldb/include/lldb/Symbol/ObjectFile.h

@@ -63,16 +63,22 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
 public:
   enum Type {
     eTypeInvalid = 0,
-    eTypeCoreFile,      /// A core file that has a checkpoint of a program's
-                        /// execution state
-    eTypeExecutable,    /// A normal executable
-    eTypeDebugInfo,     /// An object file that contains only debug information
-    eTypeDynamicLinker, /// The platform's dynamic linker executable
-    eTypeObjectFile,    /// An intermediate object file
-    eTypeSharedLibrary, /// A shared library that can be used during execution
-    eTypeStubLibrary, /// A library that can be linked against but not used for
-                      /// execution
-    eTypeJIT, /// JIT code that has symbols, sections and possibly debug info
+    /// A core file that has a checkpoint of a program's execution state.
+    eTypeCoreFile,
+    /// A normal executable.
+    eTypeExecutable,
+    /// An object file that contains only debug information.
+    eTypeDebugInfo,
+    /// The platform's dynamic linker executable.
+    eTypeDynamicLinker,
+    /// An intermediate object file.
+    eTypeObjectFile,
+    /// A shared library that can be used during execution.
+    eTypeSharedLibrary,
+    /// A library that can be linked against but not used for execution.
+    eTypeStubLibrary,
+    /// JIT code that has symbols, sections and possibly debug info.
+    eTypeJIT,
     eTypeUnknown
   };
 
@@ -201,7 +207,7 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
   ///     \b false otherwise and \a archive_file and \a archive_object
   ///     are guaranteed to be remain unchanged.
   static bool SplitArchivePathWithObject(
-      const char *path_with_object, lldb_private::FileSpec &archive_file,
+      llvm::StringRef path_with_object, lldb_private::FileSpec &archive_file,
       lldb_private::ConstString &archive_object, bool must_exist);
 
   // LLVM RTTI support

lldb/include/lldb/Symbol/Symbol.h

@@ -24,9 +24,8 @@ class Symbol : public SymbolContextScope {
   // drastically different meanings and sorting requirements.
   Symbol();
 
-  Symbol(uint32_t symID, const char *name, bool name_is_mangled,
-         lldb::SymbolType type, bool external, bool is_debug,
-         bool is_trampoline, bool is_artificial,
+  Symbol(uint32_t symID, llvm::StringRef name, lldb::SymbolType type,
+         bool external, bool is_debug, bool is_trampoline, bool is_artificial,
          const lldb::SectionSP &section_sp, lldb::addr_t value,
          lldb::addr_t size, bool size_is_valid,
          bool contains_linker_annotations, uint32_t flags);

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/Makefile

@@ -0,0 +1,7 @@
+# There is no guaranteed order in which the linker will order these
+# files, so we just have a lot of them to make it unlikely that we hit
+# the right one first by pure luck.
+
+CXX_SOURCES := main.cpp a.cpp b.cpp c.cpp d.cpp e.cpp f.cpp g.cpp
+
+include Makefile.rules

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/TestCPPAccelerator.py

@@ -0,0 +1,31 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class CPPAcceleratorTableTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipUnlessDarwin
+    @skipIf(debug_info=no_match(["dwarf"]))
+    def test(self):
+        """Test that type lookups fail early (performance)"""
+        self.build()
+        logfile = self.getBuildArtifact('dwarf.log')
+        self.expect('log enable dwarf lookups -f' + logfile)
+        target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
+            self, 'break here', lldb.SBFileSpec('main.cpp'))
+        # Pick one from the middle of the list to have a high chance
+        # of it not being in the first file looked at.
+        self.expect('frame variable inner_d')
+
+        log = open(logfile, 'r')
+        n = 0
+        for line in log:
+            if re.findall(r'[abcdefg]\.o: FindByNameAndTag\(\)', line):
+                self.assertTrue("d.o" in line)
+                n += 1
+
+        self.assertEqual(n, 1, log)

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/a.cpp

@@ -0,0 +1,2 @@
+#include "source.h"
+CLASS(A)

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/b.cpp

@@ -0,0 +1,2 @@
+#include "source.h"
+CLASS(B)

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/c.cpp

@@ -0,0 +1,2 @@
+#include "source.h"
+CLASS(C)

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/d.cpp

@@ -0,0 +1,2 @@
+#include "source.h"
+CLASS(D)

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/e.cpp

@@ -0,0 +1,2 @@
+#include "source.h"
+CLASS(E)

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/f.cpp

@@ -0,0 +1,2 @@
+#include "source.h"
+CLASS(F)

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/g.cpp

@@ -0,0 +1,2 @@
+#include "source.h"
+CLASS(G)

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/main.cpp

@@ -0,0 +1,28 @@
+#define CLASS(NAME)                             \
+  class NAME {                                  \
+  public:                                       \
+    struct Inner;                               \
+    Inner *i = nullptr;                         \
+  };                                            \
+NAME::Inner &getInner##NAME();
+
+CLASS(A)
+CLASS(B)
+CLASS(C)
+CLASS(D)
+CLASS(E)
+CLASS(F)
+CLASS(G)
+
+int main()
+{
+  A::Inner &inner_a = getInnerA();
+  B::Inner &inner_b = getInnerB();
+  C::Inner &inner_c = getInnerC();
+  D::Inner &inner_d = getInnerD();
+  E::Inner &inner_e = getInnerE();
+  F::Inner &inner_f = getInnerF();
+  G::Inner &inner_g = getInnerG();
+
+  return 0; // break here
+}

lldb/packages/Python/lldbsuite/test/lang/cpp/accelerator-table/source.h

@@ -0,0 +1,12 @@
+#define CLASS(NAME)                             \
+  class NAME {                                  \
+    public:                                     \
+    class Inner {                               \
+      int j = #NAME[0];                         \
+    };                                          \
+    Inner *i = nullptr;                         \
+  };                                            \
+                                                \
+  static NAME::Inner inner;                     \
+  static NAME obj;                              \
+  NAME::Inner &getInner##NAME() { return inner; }

lldb/source/API/SBType.cpp

@@ -799,7 +799,7 @@ const char *SBTypeMemberFunction::GetDemangledName() {
   if (m_opaque_sp) {
     ConstString mangled_str = m_opaque_sp->GetMangledName();
     if (mangled_str) {
-      Mangled mangled(mangled_str, true);
+      Mangled mangled(mangled_str);
       return mangled.GetDemangledName(mangled.GuessLanguage()).GetCString();
     }
   }

lldb/source/Core/Mangled.cpp

@@ -124,21 +124,6 @@ get_demangled_name_without_arguments(ConstString mangled,
 }
 
 #pragma mark Mangled
-// Default constructor
-Mangled::Mangled() : m_mangled(), m_demangled() {}
-
-// Constructor with an optional string and a boolean indicating if it is the
-// mangled version.
-Mangled::Mangled(ConstString s, bool mangled)
-    : m_mangled(), m_demangled() {
-  if (s)
-    SetValue(s, mangled);
-}
-
-Mangled::Mangled(llvm::StringRef name, bool is_mangled) {
-  if (!name.empty())
-    SetValue(ConstString(name), is_mangled);
-}
 
 Mangled::Mangled(ConstString s) : m_mangled(), m_demangled() {
   if (s)
@@ -150,9 +135,6 @@ Mangled::Mangled(llvm::StringRef name) {
     SetValue(ConstString(name));
 }
 
-// Destructor
-Mangled::~Mangled() {}
-
 // Convert to pointer operator. This allows code to check any Mangled objects
 // to see if they contain anything valid using code such as:
 //

lldb/source/Expression/IRExecutionUnit.cpp

@@ -669,7 +669,7 @@ FindBestAlternateMangledName(ConstString demangled,
   std::vector<ConstString> param_matches;
   for (size_t i = 0; i < alternates.size(); i++) {
     ConstString alternate_mangled_name = alternates[i];
-    Mangled mangled(alternate_mangled_name, true);
+    Mangled mangled(alternate_mangled_name);
     ConstString demangled = mangled.GetDemangledName(lang_type);
 
     CPlusPlusLanguage::MethodName alternate_cpp_name(demangled);
@@ -717,7 +717,7 @@ void IRExecutionUnit::CollectCandidateCPlusPlusNames(
     ConstString name = C_spec.name;
 
     if (CPlusPlusLanguage::IsCPPMangledName(name.GetCString())) {
-      Mangled mangled(name, true);
+      Mangled mangled(name);
       ConstString demangled =
           mangled.GetDemangledName(lldb::eLanguageTypeC_plus_plus);
 

lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp

@@ -360,7 +360,7 @@ class CommandObjectMultiwordItaniumABI_Demangle : public CommandObjectParsed {
       if (name.startswith("__Z"))
         name = name.drop_front();
 
-      Mangled mangled(name, true);
+      Mangled mangled(name);
       if (mangled.GuessLanguage() == lldb::eLanguageTypeC_plus_plus) {
         ConstString demangled(
             mangled.GetDisplayDemangledName(lldb::eLanguageTypeC_plus_plus));

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

@@ -2146,16 +2146,14 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
 
     bool is_global = symbol.getBinding() == STB_GLOBAL;
     uint32_t flags = symbol.st_other << 8 | symbol.st_info | additional_flags;
-    bool is_mangled = (symbol_name[0] == '_' && symbol_name[1] == 'Z');
-
     llvm::StringRef symbol_ref(symbol_name);
 
     // Symbol names may contain @VERSION suffixes. Find those and strip them
     // temporarily.
     size_t version_pos = symbol_ref.find('@');
     bool has_suffix = version_pos != llvm::StringRef::npos;
     llvm::StringRef symbol_bare = symbol_ref.substr(0, version_pos);
-    Mangled mangled(ConstString(symbol_bare), is_mangled);
+    Mangled mangled(symbol_bare);
 
     // Now append the suffix back to mangled and unmangled names. Only do it if
     // the demangling was successful (string is not empty).
@@ -2386,14 +2384,11 @@ static unsigned ParsePLTRelocations(
       break;
 
     const char *symbol_name = strtab_data.PeekCStr(symbol.st_name);
-    bool is_mangled =
-        symbol_name ? (symbol_name[0] == '_' && symbol_name[1] == 'Z') : false;
     uint64_t plt_index = plt_offset + i * plt_entsize;
 
     Symbol jump_symbol(
         i + start_id,          // Symbol table index
         symbol_name,           // symbol name.
-        is_mangled,            // is the symbol name mangled?
         eSymbolTypeTrampoline, // Type of this symbol
         false,                 // Is this globally visible?
         false,                 // Is this symbol debug info?
@@ -2781,7 +2776,6 @@ void ObjectFileELF::ParseUnwindSymbols(Symtab *symbol_table,
         Symbol eh_symbol(
             symbol_id,       // Symbol table index.
             symbol_name,     // Symbol name.
-            false,           // Is the symbol name mangled?
             eSymbolTypeCode, // Type of this symbol.
             true,            // Is this globally visible?
             false,           // Is this symbol debug info?

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

@@ -1895,18 +1895,6 @@ class MachSymtabSectionInfo {
 };
 
 struct TrieEntry {
-  TrieEntry()
-      : name(), address(LLDB_INVALID_ADDRESS), flags(0), other(0),
-        import_name() {}
-
-  void Clear() {
-    name.Clear();
-    address = LLDB_INVALID_ADDRESS;
-    flags = 0;
-    other = 0;
-    import_name.Clear();
-  }
-
   void Dump() const {
     printf("0x%16.16llx 0x%16.16llx 0x%16.16llx \"%s\"",
            static_cast<unsigned long long>(address),
@@ -1918,9 +1906,9 @@ struct TrieEntry {
       printf("\n");
   }
   ConstString name;
-  uint64_t address;
-  uint64_t flags;
-  uint64_t other;
+  uint64_t address = LLDB_INVALID_ADDRESS;
+  uint64_t flags = 0;
+  uint64_t other = 0;
   ConstString import_name;
 };
 

lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

@@ -350,8 +350,8 @@ void SymbolFileBreakpad::AddSymbols(Symtab &symtab) {
       return;
     }
     symbols.try_emplace(
-        address, /*symID*/ 0, Mangled(name, /*is_mangled*/ false),
-        eSymbolTypeCode, /*is_global*/ true, /*is_debug*/ false,
+        address, /*symID*/ 0, Mangled(name), eSymbolTypeCode,
+        /*is_global*/ true, /*is_debug*/ false,
         /*is_trampoline*/ false, /*is_artificial*/ false,
         AddressRange(section_sp, address - section_sp->GetFileAddress(),
                      size.getValueOr(0)),

lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp

@@ -110,6 +110,7 @@ void AppleDWARFIndex::GetTypes(const DWARFDeclContext &context,
   const bool has_qualified_name_hash =
       m_apple_types_up->GetHeader().header_data.ContainsAtom(
           DWARFMappedHash::eAtomTypeQualNameHash);
+
   const ConstString type_name(context[0].name);
   const dw_tag_t tag = context[0].tag;
   if (has_tag && has_qualified_name_hash) {
@@ -119,12 +120,32 @@ void AppleDWARFIndex::GetTypes(const DWARFDeclContext &context,
       m_module.LogMessage(log, "FindByNameAndTagAndQualifiedNameHash()");
     m_apple_types_up->FindByNameAndTagAndQualifiedNameHash(
         type_name.GetStringRef(), tag, qualified_name_hash, offsets);
-  } else if (has_tag) {
+    return;
+  }
+
+  if (has_tag) {
+    // When searching for a scoped type (for example,
+    // "std::vector<int>::const_iterator") searching for the innermost
+    // name alone ("const_iterator") could yield many false
+    // positives. By searching for the parent type ("vector<int>")
+    // first we can avoid extracting type DIEs from object files that
+    // would fail the filter anyway.
+    if (!has_qualified_name_hash && (context.GetSize() > 1) &&
+        (context[1].tag == DW_TAG_class_type ||
+         context[1].tag == DW_TAG_structure_type)) {
+      DIEArray class_matches;
+      m_apple_types_up->FindByName(context[1].name, class_matches);
+      if (class_matches.empty())
+        return;
+    }
+
     if (log)
       m_module.LogMessage(log, "FindByNameAndTag()");
     m_apple_types_up->FindByNameAndTag(type_name.GetStringRef(), tag, offsets);
-  } else
-    m_apple_types_up->FindByName(type_name.GetStringRef(), offsets);
+    return;
+  }
+
+  m_apple_types_up->FindByName(type_name.GetStringRef(), offsets);
 }
 
 void AppleDWARFIndex::GetNamespaces(ConstString name, DIEArray &offsets) {