shafik created this revision. shafik added reviewers: jingham, jasonmolenda, aprantl. Herald added a reviewer: EricWF. Herald added subscribers: ldionne, christof.
Performance issues lead to the libc++ `std::function` formatter to be disabled, see D65666 <https://reviews.llvm.org/D65666> 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. We are also expanding the tests but they will remain disabled. https://reviews.llvm.org/D67111 Files: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h =================================================================== --- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h +++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h @@ -9,7 +9,9 @@ #ifndef liblldb_CPPLanguageRuntime_h_ #define liblldb_CPPLanguageRuntime_h_ +#include <unordered_map> #include <vector> + #include "lldb/Core/PluginInterface.h" #include "lldb/Target/LanguageRuntime.h" #include "lldb/lldb-private.h" @@ -82,6 +84,12 @@ CPPLanguageRuntime(Process *process); private: + using OperatorStringToCallableInfoMap = + std::unordered_map<std::string, + CPPLanguageRuntime::LibCppStdFunctionCallableInfo>; + + OperatorStringToCallableInfoMap CallableLookupCache; + DISALLOW_COPY_AND_ASSIGN(CPPLanguageRuntime); }; Index: source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp =================================================================== --- source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp +++ source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp @@ -193,13 +193,30 @@ symbol = sc.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") && + !first_template_parameter.contains("$_") && + !first_template_parameter.contains("'lambda'") && + !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]() { // Given case 1: // // main::$_0 + // Bar::add_num2(int)::'lambda'(int) // // we want to append ::operator()() - if (first_template_parameter.contains("$_")) + if (first_template_parameter.contains("$_") || + first_template_parameter.contains("'lambda'")) return llvm::Regex::escape(first_template_parameter.str()) + R"(::operator\(\)\(.*\))"; @@ -228,6 +245,9 @@ std::string func_to_match = get_name(); + if (CallableLookupCache.count(func_to_match)) + return CallableLookupCache[func_to_match]; + SymbolContextList scl; target.GetImages().FindSymbolsMatchingRegExAndType( @@ -249,6 +269,7 @@ addr.CalculateSymbolContextLineEntry(line_entry); if (first_template_parameter.contains("$_") || + first_template_parameter.contains("'lambda'") || (symbol != nullptr && symbol->GetName().GetStringRef().contains("__invoke"))) { // Case 1 and 2 @@ -262,19 +283,10 @@ 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; } Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp =================================================================== --- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp +++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/main.cpp @@ -17,8 +17,25 @@ 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 @@ 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. } Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py =================================================================== --- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py +++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py @@ -41,15 +41,38 @@ substrs=['stopped', 'stop reason = breakpoint']) + # We are running the lookups twice in some cases because now we are + # caching the results for some cases mainly lambdas and callable objects. + # We want to validate that subsequent lookups obtain the same result. + self.expect("frame variable foo2_f", + substrs=['foo2_f = Lambda in File main.cpp at Line 30']) + self.expect("frame variable foo2_f", + substrs=['foo2_f = Lambda in File main.cpp at Line 30']) + + lldbutil.continue_to_breakpoint(self.process(), bkpt) + + self.expect("frame variable add_num2_f", + substrs=['add_num2_f = Lambda in File main.cpp at Line 21']) + self.expect("frame variable add_num2_f", + substrs=['add_num2_f = Lambda in File main.cpp at Line 21']) + + lldbutil.continue_to_breakpoint(self.process(), bkpt) + self.expect("frame variable f1", substrs=['f1 = Function = foo(int, int)']) self.expect("frame variable f2", - substrs=['f2 = Lambda in File main.cpp at Line 26']) + substrs=['f2 = Lambda in File main.cpp at Line 43']) + self.expect("frame variable f2", + substrs=['f2 = Lambda in File main.cpp at Line 43']) self.expect("frame variable f3", - substrs=['f3 = Lambda in File main.cpp at Line 30']) + substrs=['f3 = Lambda in File main.cpp at Line 47']) + self.expect("frame variable f3", + substrs=['f3 = Lambda in File main.cpp at Line 47']) + self.expect("frame variable f4", + substrs=['f4 = Function in File main.cpp at Line 16']) self.expect("frame variable f4", substrs=['f4 = Function in File main.cpp at Line 16'])
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits