Author: shafik Date: 2019-11-06T16:02:56-08:00 New Revision: e18f4db208baa84800cf304d7e15f2ee7343cd05
URL: https://github.com/llvm/llvm-project/commit/e18f4db208baa84800cf304d7e15f2ee7343cd05 DIFF: https://github.com/llvm/llvm-project/commit/e18f4db208baa84800cf304d7e15f2ee7343cd05.diff LOG: [LLDB] Adding caching to libc++ std::function formatter for lookups that require scanning symbols Performance issues lead to the libc++ std::function formatter to be disabled. This change is the first of two changes that should address the performance issues and allow us to enable the formatter again. In some cases we end up scanning the symbol table for the callable wrapped by std::function for those cases we will now cache the results and used the cache in subsequent look-ups. This still leaves a large cost for the initial lookup which will be addressed in the next change. Differential Revision: https://reviews.llvm.org/D67111 Added: Modified: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h Removed: ################################################################################ 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 f06ab5d70ba3..a0cf19b3bed6 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 @@ -15,11 +15,22 @@ class LibCxxFunctionTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) - def get_variable(self, name): - var = self.frame().FindVariable(name) - var.SetPreferDynamicValue(lldb.eDynamicCanRunTarget) - var.SetPreferSyntheticValue(True) - return var + # 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): + 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"]) + + 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"]) + # 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. @@ -41,17 +52,22 @@ def test(self): substrs=['stopped', 'stop reason = breakpoint']) - self.expect("frame variable f1", - substrs=['f1 = Function = foo(int, int)']) + self.run_frame_var_check_cache_use("foo2_f", "Lambda in File main.cpp at Line 30") + + lldbutil.continue_to_breakpoint(self.process(), bkpt) - self.expect("frame variable f2", - substrs=['f2 = Lambda in File main.cpp at Line 26']) + self.run_frame_var_check_cache_use("add_num2_f", "Lambda in File main.cpp at Line 21") - self.expect("frame variable f3", - substrs=['f3 = Lambda in File main.cpp at Line 30']) + lldbutil.continue_to_breakpoint(self.process(), bkpt) - self.expect("frame variable f4", - substrs=['f4 = Function in File main.cpp at Line 16']) + 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") + + # These cases won't hit the cache at all but also don't require + # an expensive lookup. + self.expect("frame variable f1", + substrs=['f1 = Function = foo(int, int)']) self.expect("frame variable f5", substrs=['f5 = Function = Bar::add_num(int) const']) diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp index b78161df2c1e..d0c931ddc8b4 100644 --- a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp +++ b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp @@ -17,8 +17,25 @@ struct Bar { return 66 ; } int add_num(int i) const { return i + 3 ; } + int add_num2(int i) { + std::function<int (int)> add_num2_f = [](int x) { + return x+1; + }; + + return add_num2_f(i); // Set break point at this line. + } } ; +int foo2() { + auto f = [](int x) { + return x+1; + }; + + std::function<int (int)> foo2_f = f; + + return foo2_f(10); // Set break point at this line. +} + int main (int argc, char *argv[]) { int acc = 42; @@ -35,5 +52,8 @@ int main (int argc, char *argv[]) std::function<int ()> f4( bar1 ) ; std::function<int (const Bar&, int)> f5 = &Bar::add_num; + int foo2_result = foo2(); + int bar_add_num2_result = bar1.add_num2(10); + return f1(acc,acc) + f2(acc) + f3(acc+1,acc+2) + f4() + f5(bar1, 10); // Set break point at this line. } diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp index f38014505a8b..379b0ba1b4d9 100644 --- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp @@ -192,14 +192,33 @@ 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'"); + }; - auto get_name = [&first_template_parameter, &symbol]() { + // 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")) { + optional_info.callable_case = + LibCppStdFunctionCallableCase::FreeOrMemberFunction; + optional_info.callable_address = function_address_resolved; + optional_info.callable_symbol = *symbol; + + 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 (first_template_parameter.contains("$_")) + if (contains_lambda_identifier(first_template_parameter)) return llvm::Regex::escape(first_template_parameter.str()) + R"(::operator\(\)\(.*\))"; @@ -228,6 +247,10 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo( std::string func_to_match = get_name(); + auto it = CallableLookupCache.find(func_to_match); + if (it != CallableLookupCache.end()) + return it->second; + SymbolContextList scl; target.GetImages().FindSymbolsMatchingRegExAndType( @@ -248,7 +271,7 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo( LineEntry line_entry; addr.CalculateSymbolContextLineEntry(line_entry); - if (first_template_parameter.contains("$_") || + if (contains_lambda_identifier(first_template_parameter) || (symbol != nullptr && symbol->GetName().GetStringRef().contains("__invoke"))) { // Case 1 and 2 @@ -262,19 +285,10 @@ CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo( optional_info.callable_symbol = *symbol; optional_info.callable_line_entry = line_entry; optional_info.callable_address = addr; - return optional_info; } } - // Case 4 or 5 - if (symbol && !symbol->GetName().GetStringRef().startswith("vtable for")) { - optional_info.callable_case = - LibCppStdFunctionCallableCase::FreeOrMemberFunction; - optional_info.callable_address = function_address_resolved; - optional_info.callable_symbol = *symbol; - - return optional_info; - } + CallableLookupCache[func_to_match] = optional_info; return optional_info; } diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h index 28526361efc4..abdd79fcd7b9 100644 --- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h +++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h @@ -10,6 +10,9 @@ #define liblldb_CPPLanguageRuntime_h_ #include <vector> + +#include "llvm/ADT/StringMap.h" + #include "lldb/Core/PluginInterface.h" #include "lldb/Target/LanguageRuntime.h" #include "lldb/lldb-private.h" @@ -82,6 +85,11 @@ class CPPLanguageRuntime : public LanguageRuntime { CPPLanguageRuntime(Process *process); private: + using OperatorStringToCallableInfoMap = + llvm::StringMap<CPPLanguageRuntime::LibCppStdFunctionCallableInfo>; + + OperatorStringToCallableInfoMap CallableLookupCache; + DISALLOW_COPY_AND_ASSIGN(CPPLanguageRuntime); }; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
