[Lldb-commits] [lldb] 91e94a7 - [LLDB][Formatters] Re-enable std::function formatter with fixes to improve non-cached lookup performance
via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 12 11:30:28 PST 2019
Author: shafik
Date: 2019-11-12T11:30:18-08:00
New Revision: 91e94a7015f14f78809e875c43acbd341d081479
URL: https://github.com/llvm/llvm-project/commit/91e94a7015f14f78809e875c43acbd341d081479
DIFF: https://github.com/llvm/llvm-project/commit/91e94a7015f14f78809e875c43acbd341d081479.diff
LOG: [LLDB][Formatters] Re-enable std::function formatter with fixes to improve non-cached lookup performance
Performance issues lead to the libc++ std::function formatter to be disabled. We addressed some of those performance issues by adding caching see D67111
This PR fixes the first lookup performance by not using FindSymbolsMatchingRegExAndType(...) and instead finding the compilation unit the std::function wrapped callable should be in and then searching for the callable directly in the CU.
Differential Revision: https://reviews.llvm.org/D69913
Added:
Modified:
lldb/include/lldb/Symbol/CompileUnit.h
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Symbol/CompileUnit.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Symbol/CompileUnit.h b/lldb/include/lldb/Symbol/CompileUnit.h
index d132c367b999..4a6306fcf4a7 100644
--- a/lldb/include/lldb/Symbol/CompileUnit.h
+++ b/lldb/include/lldb/Symbol/CompileUnit.h
@@ -163,6 +163,18 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
void ForeachFunction(
llvm::function_ref<bool(const lldb::FunctionSP &)> lambda) const;
+ /// Find a function in the compile unit based on the predicate matching_lambda
+ ///
+ /// \param[in] matching_lambda
+ /// A predicate that will be used within FindFunction to evaluate each
+ /// FunctionSP in m_functions_by_uid. When the predicate returns true
+ /// FindFunction will return the corresponding FunctionSP.
+ ///
+ /// \return
+ /// The first FunctionSP that the matching_lambda prediate returns true for.
+ lldb::FunctionSP FindFunction(
+ llvm::function_ref<bool(const lldb::FunctionSP &)> matching_lambda);
+
/// Dump the compile unit contents to the stream \a s.
///
/// \param[in] s
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
index a0cf19b3bed6..ea03084cbe60 100644
--- a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
+++ b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
@@ -17,24 +17,22 @@ class LibCxxFunctionTestCase(TestBase):
# Run frame var for a variable twice. Verify we do not hit the cache
# the first time but do the second time.
- def run_frame_var_check_cache_use(self, variable, result_to_match):
+ def run_frame_var_check_cache_use(self, variable, result_to_match, skip_find_function=False):
self.runCmd("log timers reset")
self.expect("frame variable " + variable,
substrs=[variable + " = " + result_to_match])
- self.expect("log timers dump",
- substrs=["lldb_private::Module::FindSymbolsMatchingRegExAndType"])
+ if not skip_find_function:
+ self.expect("log timers dump",
+ substrs=["lldb_private::CompileUnit::FindFunction"])
self.runCmd("log timers reset")
self.expect("frame variable " + variable,
substrs=[variable + " = " + result_to_match])
self.expect("log timers dump",
matching=False,
- substrs=["lldb_private::Module::FindSymbolsMatchingRegExAndType"])
+ substrs=["lldb_private::CompileUnit::FindFunction"])
- # Temporarily skipping for everywhere b/c we are disabling the std::function formatter
- # due to performance issues but plan on turning it back on once they are addressed.
- @skipIf
@add_test_categories(["libc++"])
def test(self):
"""Test that std::function as defined by libc++ is correctly printed by LLDB"""
@@ -61,8 +59,10 @@ def test(self):
lldbutil.continue_to_breakpoint(self.process(), bkpt)
self.run_frame_var_check_cache_use("f2", "Lambda in File main.cpp at Line 43")
- self.run_frame_var_check_cache_use("f3", "Lambda in File main.cpp at Line 47")
- self.run_frame_var_check_cache_use("f4", "Function in File main.cpp at Line 16")
+ self.run_frame_var_check_cache_use("f3", "Lambda in File main.cpp at Line 47", True)
+ # TODO reenable this case when std::function formatter supports
+ # general callable object case.
+ #self.run_frame_var_check_cache_use("f4", "Function in File main.cpp at Line 16")
# These cases won't hit the cache at all but also don't require
# an expensive lookup.
diff --git a/lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py b/lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
index 3d2e03219834..348d03ead534 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
+++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
@@ -59,10 +59,12 @@ def test(self):
self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetFileSpec().GetFilename(), self.main_source) ;
process.Continue()
- thread.StepInto()
- self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), self.source_bar_operator_line ) ;
- self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetFileSpec().GetFilename(), self.main_source) ;
- process.Continue()
+ # TODO reenable this case when std::function formatter supports
+ # general callable object case.
+ #thread.StepInto()
+ #self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), self.source_bar_operator_line ) ;
+ #self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetFileSpec().GetFilename(), self.main_source) ;
+ #process.Continue()
thread.StepInto()
self.assertEqual( thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), self.source_bar_add_num_line ) ;
diff --git a/lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp b/lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
index a85e77db040e..ebbb05e6d138 100644
--- a/lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
+++ b/lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
@@ -32,7 +32,9 @@ int main (int argc, char *argv[])
return f_mem(bar1) + // Set break point at this line.
f1(acc,acc) + // Source main invoking f1
f2(acc) + // Set break point at this line.
- f3(acc+1,acc+2) + // Set break point at this line.
- f4() + // Set break point at this line.
+ f3(acc+1,acc+2) + // Set break point at this line.
+ // TODO reenable this case when std::function formatter supports
+ // general callable object case.
+ //f4() + // Set break point at this line.
f5(bar1, 10); // Set break point at this line.
}
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 5886a78a5ef0..bf85bf6796d4 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -567,6 +567,11 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
"weak_ptr synthetic children",
ConstString("^(std::__[[:alnum:]]+::)weak_ptr<.+>(( )?&)?$"),
stl_synth_flags, true);
+ AddCXXSummary(cpp_category_sp,
+ lldb_private::formatters::LibcxxFunctionSummaryProvider,
+ "libc++ std::function summary provider",
+ ConstString("^std::__[[:alnum:]]+::function<.+>$"),
+ stl_summary_flags, true);
stl_summary_flags.SetDontShowChildren(false);
stl_summary_flags.SetSkipPointers(false);
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index 379b0ba1b4d9..b4d8ba2218a1 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -21,6 +21,7 @@
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/UniqueCStringMap.h"
#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/CompileUnit.h"
#include "lldb/Target/ABI.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/RegisterContext.h"
@@ -28,6 +29,7 @@
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/ThreadPlanRunToAddress.h"
#include "lldb/Target/ThreadPlanStepInRange.h"
+#include "lldb/Utility/Timer.h"
using namespace lldb;
using namespace lldb_private;
@@ -58,9 +60,53 @@ bool CPPLanguageRuntime::GetObjectDescription(
return false;
}
+bool contains_lambda_identifier(llvm::StringRef &str_ref) {
+ return str_ref.contains("$_") || str_ref.contains("'lambda'");
+};
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo
+line_entry_helper(Target &target, const SymbolContext &sc, Symbol *symbol,
+ llvm::StringRef first_template_param_sref,
+ bool has___invoke) {
+
+ CPPLanguageRuntime::LibCppStdFunctionCallableInfo optional_info;
+
+ AddressRange range;
+ sc.GetAddressRange(eSymbolContextEverything, 0, false, range);
+
+ Address address = range.GetBaseAddress();
+
+ Address addr;
+ if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
+ addr)) {
+ LineEntry line_entry;
+ addr.CalculateSymbolContextLineEntry(line_entry);
+
+ if (contains_lambda_identifier(first_template_param_sref) || has___invoke) {
+ // Case 1 and 2
+ optional_info.callable_case = lldb_private::CPPLanguageRuntime::
+ LibCppStdFunctionCallableCase::Lambda;
+ } else {
+ // Case 3
+ optional_info.callable_case = lldb_private::CPPLanguageRuntime::
+ LibCppStdFunctionCallableCase::CallableObject;
+ }
+
+ optional_info.callable_symbol = *symbol;
+ optional_info.callable_line_entry = line_entry;
+ optional_info.callable_address = addr;
+ }
+
+ return optional_info;
+}
+
CPPLanguageRuntime::LibCppStdFunctionCallableInfo
CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
lldb::ValueObjectSP &valobj_sp) {
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scoped_timer(func_cat,
+ "CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo");
+
LibCppStdFunctionCallableInfo optional_info;
if (!valobj_sp)
@@ -93,7 +139,7 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
// this entry and lookup operator()() and obtain the line table entry.
// 3) a callable object via operator()(). We will obtain the name of the
// object from the first template parameter from __func's vtable. We will
- // look up the objectc operator()() and obtain the line table entry.
+ // look up the objects operator()() and obtain the line table entry.
// 4) a member function. A pointer to the function will stored after the
// we will obtain the name from this pointer.
// 5) a free function. A pointer to the function will stored after the vtable
@@ -113,6 +159,9 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
optional_info.member__f_pointer_value = member__f_pointer_value;
+ if (!member__f_pointer_value)
+ return optional_info;
+
ExecutionContext exe_ctx(valobj_sp->GetExecutionContextRef());
Process *process = exe_ctx.GetProcessPtr();
@@ -130,8 +179,14 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
if (status.Fail())
return optional_info;
+ lldb::addr_t vtable_address_first_entry =
+ process->ReadPointerFromMemory(vtable_address + address_size, status);
+
+ if (status.Fail())
+ return optional_info;
+
lldb::addr_t address_after_vtable = member__f_pointer_value + address_size;
- // As commened above we may not have a function pointer but if we do we will
+ // As commented above we may not have a function pointer but if we do we will
// need it.
lldb::addr_t possible_function_address =
process->ReadPointerFromMemory(address_after_vtable, status);
@@ -144,9 +199,15 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
if (target.GetSectionLoadList().IsEmpty())
return optional_info;
+ Address vtable_first_entry_resolved;
+
+ if (!target.GetSectionLoadList().ResolveLoadAddress(
+ vtable_address_first_entry, vtable_first_entry_resolved))
+ return optional_info;
+
Address vtable_addr_resolved;
SymbolContext sc;
- Symbol *symbol;
+ Symbol *symbol = nullptr;
if (!target.GetSectionLoadList().ResolveLoadAddress(vtable_address,
vtable_addr_resolved))
@@ -159,7 +220,7 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
if (symbol == nullptr)
return optional_info;
- llvm::StringRef vtable_name(symbol->GetName().GetCString());
+ llvm::StringRef vtable_name(symbol->GetName().GetStringRef());
bool found_expected_start_string =
vtable_name.startswith("vtable for std::__1::__function::__func<");
@@ -172,6 +233,11 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
// ... __func<main::$_0, std::__1::allocator<main::$_0> ...
// ^^^^^^^^^
//
+ // We could see names such as:
+ // main::$_0
+ // Bar::add_num2(int)::'lambda'(int)
+ // Bar
+ //
// We do this by find the first < and , and extracting in between.
//
// This covers the case of the lambda known at compile time.
@@ -192,17 +258,29 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
function_address_resolved, eSymbolContextEverything, sc);
symbol = sc.symbol;
}
-
- auto contains_lambda_identifier = []( llvm::StringRef & str_ref ) {
- return str_ref.contains("$_") || str_ref.contains("'lambda'");
- };
+
+ // These conditions are used several times to simplify statements later on.
+ bool has___invoke =
+ (symbol ? symbol->GetName().GetStringRef().contains("__invoke") : false);
+ auto calculate_symbol_context_helper = [](auto &t,
+ SymbolContextList &sc_list) {
+ SymbolContext sc;
+ t->CalculateSymbolContext(&sc);
+ sc_list.Append(sc);
+ };
+
+ // Case 2
+ if (has___invoke) {
+ SymbolContextList scl;
+ calculate_symbol_context_helper(symbol, scl);
+
+ return line_entry_helper(target, scl[0], symbol, first_template_parameter,
+ has___invoke);
+ }
// Case 4 or 5
- // We eliminate these cases early because they don't need the potentially
- // expensive lookup through the symbol table.
if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for") &&
- !contains_lambda_identifier(first_template_parameter) &&
- !symbol->GetName().GetStringRef().contains("__invoke")) {
+ !contains_lambda_identifier(first_template_parameter) && !has___invoke) {
optional_info.callable_case =
LibCppStdFunctionCallableCase::FreeOrMemberFunction;
optional_info.callable_address = function_address_resolved;
@@ -211,41 +289,7 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
return optional_info;
}
- auto get_name = [&first_template_parameter, &symbol, contains_lambda_identifier]() {
- // Given case 1:
- //
- // main::$_0
- // Bar::add_num2(int)::'lambda'(int)
- //
- // we want to append ::operator()()
- if (contains_lambda_identifier(first_template_parameter))
- return llvm::Regex::escape(first_template_parameter.str()) +
- R"(::operator\(\)\(.*\))";
-
- if (symbol != nullptr &&
- symbol->GetName().GetStringRef().contains("__invoke")) {
-
- llvm::StringRef symbol_name = symbol->GetName().GetStringRef();
- size_t pos2 = symbol_name.find_last_of(':');
-
- // Given case 2:
- //
- // main::$_1::__invoke(...)
- //
- // We want to slice off __invoke(...) and append operator()()
- std::string lambda_operator =
- llvm::Regex::escape(symbol_name.slice(0, pos2 + 1).str()) +
- R"(operator\(\)\(.*\))";
-
- return lambda_operator;
- }
-
- // Case 3
- return first_template_parameter.str() + R"(::operator\(\)\(.*\))";
- ;
- };
-
- std::string func_to_match = get_name();
+ std::string func_to_match = first_template_parameter.str();
auto it = CallableLookupCache.find(func_to_match);
if (it != CallableLookupCache.end())
@@ -253,41 +297,40 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
SymbolContextList scl;
- target.GetImages().FindSymbolsMatchingRegExAndType(
- RegularExpression{R"(^)" + func_to_match}, eSymbolTypeAny, scl);
+ CompileUnit *vtable_cu =
+ vtable_first_entry_resolved.CalculateSymbolContextCompileUnit();
+ llvm::StringRef name_to_use = func_to_match;
- // Case 1,2 or 3
- if (scl.GetSize() >= 1) {
- SymbolContext sc2 = scl[0];
-
- AddressRange range;
- sc2.GetAddressRange(eSymbolContextEverything, 0, false, range);
-
- Address address = range.GetBaseAddress();
-
- Address addr;
- if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
- addr)) {
- LineEntry line_entry;
- addr.CalculateSymbolContextLineEntry(line_entry);
-
- if (contains_lambda_identifier(first_template_parameter) ||
- (symbol != nullptr &&
- symbol->GetName().GetStringRef().contains("__invoke"))) {
- // Case 1 and 2
- optional_info.callable_case = LibCppStdFunctionCallableCase::Lambda;
- } else {
- // Case 3
- optional_info.callable_case =
- LibCppStdFunctionCallableCase::CallableObject;
- }
-
- optional_info.callable_symbol = *symbol;
- optional_info.callable_line_entry = line_entry;
- optional_info.callable_address = addr;
+ // Case 3, we have a callable object instead of a lambda
+ //
+ // TODO
+ // We currently don't support this case a callable object may have multiple
+ // operator()() varying on const/non-const and number of arguments and we
+ // don't have a way to currently distinguish them so we will bail out now.
+ if (!contains_lambda_identifier(name_to_use))
+ return optional_info;
+
+ if (vtable_cu && !has___invoke) {
+ lldb::FunctionSP func_sp =
+ vtable_cu->FindFunction([name_to_use](const FunctionSP &f) {
+ auto name = f->GetName().GetStringRef();
+ if (name.startswith(name_to_use) && name.contains("operator"))
+ return true;
+
+ return false;
+ });
+
+ if (func_sp) {
+ calculate_symbol_context_helper(func_sp, scl);
}
}
+ // Case 1 or 3
+ if (scl.GetSize() >= 1) {
+ optional_info = line_entry_helper(target, scl[0], symbol,
+ first_template_parameter, has___invoke);
+ }
+
CallableLookupCache[func_to_match] = optional_info;
return optional_info;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 12ae16ee176d..dfa913280232 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -830,6 +830,8 @@ lldb::LanguageType SymbolFileDWARF::ParseLanguage(CompileUnit &comp_unit) {
}
size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) {
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scoped_timer(func_cat, "SymbolFileDWARF::ParseFunctions");
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
if (!dwarf_cu)
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index 41086d2df3df..7e2e654388af 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -12,6 +12,7 @@
#include "lldb/Symbol/SymbolFile.h"
#include "lldb/Symbol/VariableList.h"
#include "lldb/Target/Language.h"
+#include "lldb/Utility/Timer.h"
using namespace lldb;
using namespace lldb_private;
@@ -81,6 +82,31 @@ void CompileUnit::ForeachFunction(
return;
}
+lldb::FunctionSP CompileUnit::FindFunction(
+ llvm::function_ref<bool(const FunctionSP &)> matching_lambda) {
+ static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+ Timer scoped_timer(func_cat, "CompileUnit::FindFunction");
+
+ lldb::ModuleSP module = CalculateSymbolContextModule();
+
+ if (!module)
+ return {};
+
+ SymbolFile *symbol_file = module->GetSymbolFile();
+
+ if (!symbol_file)
+ return {};
+
+ // m_functions_by_uid is filled in lazily but we need all the entries.
+ symbol_file->ParseFunctions(*this);
+
+ for (auto &p : m_functions_by_uid) {
+ if (matching_lambda(p.second))
+ return p.second;
+ }
+ return {};
+}
+
// Dump the current contents of this object. No functions that cause on demand
// parsing of functions, globals, statics are called, so this is a good
// function to call to get an idea of the current contents of the CompileUnit
More information about the lldb-commits
mailing list