[Lldb-commits] [PATCH] D69843: Expression eval lookup - prune methods without parsing

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin created this revision.
jarin added a reviewer: labath.
jarin added a project: LLDB.
Herald added subscribers: lldb-commits, aprantl.

In large codebases, we sometimes see Module::FindFunctions (when called from
ClangExpressionDeclMap::FindExternalVisibleDecls) returning huge amounts of
functions. For example, a simple unreal engine example contains

> 10000 of functions with the name 'operator*'. Evaluating an expression "*x",

where x is an instance of a class, will cause all those function's decl
contexts to be parsed, taking multiple seconds. However, most of those
parsed contexts will be immediately thrown away because most of the
functions are methods.

With this patch, I am trying to avoid the parsing by checking method-ness
directly from debug info rather than from parsed contexts. This helps a lot:
our unreal engine expression evaluation pause goes roughly from 4.5s to 0.3s.

However, my patch feels wrong - ideally, we would ignore methods already
during lookup (in ManualDWARFIndex::GetFunctions).

A resonable solution would be to change the meaning of
eFunctionNameTypeFull to only mean mangled names, and if the caller
wanted methods and basenames, they would have to call with
eFunctionNameTypeFull | eFunctionNameTypeMethod |
eFunctionNameTypeBase. This would require some amount churn at
(Get|Find)Functions call sites. Also, I am not sure about implications
for Apple's lookup index.

Thoughts?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69843

Files:
  lldb/include/lldb/Symbol/Function.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Symbol/Function.cpp


Index: lldb/source/Symbol/Function.cpp
===================================================================
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -454,6 +454,16 @@
   return CompilerDeclContext();
 }
 
+bool Function::CanBeMethod() {
+  ModuleSP module_sp = CalculateSymbolContextModule();
+
+  if (module_sp) {
+    if (SymbolFile *sym_file = module_sp->GetSymbolFile())
+      return sym_file->CanBeMethod(GetID());
+  }
+  return true;
+}
+
 Type *Function::GetType() {
   if (m_type == nullptr) {
     SymbolContext sc;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -144,6 +144,8 @@
   lldb_private::CompilerDeclContext
   GetDeclContextContainingUID(lldb::user_id_t uid) override;
 
+  bool CanBeMethod(lldb::user_id_t uid) override;
+
   void
   ParseDeclsForContext(lldb_private::CompilerDeclContext decl_ctx) override;
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1314,6 +1314,16 @@
   return CompilerDeclContext();
 }
 
+bool SymbolFileDWARF::CanBeMethod(lldb::user_id_t uid) {
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
+  // Anytime we have a lldb::user_id_t, we must get the DIE by calling
+  // SymbolFileDWARF::GetDIE(). See comments inside the
+  // SymbolFileDWARF::GetDIE() for details.
+  if (DWARFDIE die = GetDIE(uid))
+    return die.IsMethod();
+  return true;
+}
+
 Type *SymbolFileDWARF::ResolveTypeUID(lldb::user_id_t type_uid) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Anytime we have a lldb::user_id_t, we must get the DIE by calling
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1292,6 +1292,8 @@
           // Filter out functions without declaration contexts, as well as
           // class/instance methods, since they'll be skipped in the code that
           // follows anyway.
+          if (function->CanBeMethod())
+            continue;
           CompilerDeclContext func_decl_context = function->GetDeclContext();
           if (!func_decl_context ||
               func_decl_context.IsClassMethod(nullptr, nullptr, nullptr))
Index: lldb/include/lldb/Symbol/SymbolFile.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolFile.h
+++ lldb/include/lldb/Symbol/SymbolFile.h
@@ -164,6 +164,7 @@
   virtual CompilerDeclContext GetDeclContextContainingUID(lldb::user_id_t uid) {
     return CompilerDeclContext();
   }
+  virtual bool CanBeMethod(lldb::user_id_t uid) { return true; }
   virtual uint32_t ResolveSymbolContext(const Address &so_addr,
                                         lldb::SymbolContextItem resolve_scope,
                                         SymbolContext &sc) = 0;
Index: lldb/include/lldb/Symbol/Function.h
===================================================================
--- lldb/include/lldb/Symbol/Function.h
+++ lldb/include/lldb/Symbol/Function.h
@@ -475,6 +475,10 @@
   ///     The DeclContext, or NULL if none exists.
   CompilerDeclContext GetDeclContext();
 
+  /// A quick test for being a method. This is used to filter out methods from
+  /// lists of functions without fully parsing debug info.
+  bool CanBeMethod();
+
   /// Get accessor for the type that describes the function return value type,
   /// and parameter types.
   ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69843.227861.patch
Type: text/x-patch
Size: 3929 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20191105/5b5afe0e/attachment.bin>