llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) <details> <summary>Changes</summary> This reduces unnecessary string allocations and copies when handling the variables request. --- Full diff: https://github.com/llvm/llvm-project/pull/172661.diff 5 Files Affected: - (modified) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+4-6) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+38-42) - (modified) lldb/tools/lldb-dap/JSONUtils.h (+6-6) - (modified) lldb/tools/lldb-dap/ProtocolUtils.cpp (+1-1) - (modified) lldb/tools/lldb-dap/ProtocolUtils.h (+5-5) ``````````diff diff --git a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp index 5fa2b1ef5e20d..0177df8b754ab 100644 --- a/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp @@ -26,9 +26,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { const uint64_t var_ref = arguments.variablesReference; const uint64_t count = arguments.count; const uint64_t start = arguments.start; - bool hex = false; - if (arguments.format) - hex = arguments.format->hex; + const bool hex = arguments.format ? arguments.format->hex : false; std::vector<Variable> variables; @@ -85,7 +83,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { const int64_t end_idx = start_idx + ((count == 0) ? num_children : count); // We first find out which variable names are duplicated - std::map<std::string, int> variable_name_counts; + std::map<llvm::StringRef, int> variable_name_counts; for (auto i = start_idx; i < end_idx; ++i) { lldb::SBValue variable = top_scope->GetValueAtIndex(i); if (!variable.IsValid()) @@ -139,7 +137,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { const bool is_permanent = dap.variables.IsPermanentVariableReference(var_ref); auto addChild = [&](lldb::SBValue child, - std::optional<std::string> custom_name = {}) { + std::optional<llvm::StringRef> custom_name = {}) { if (!child.IsValid()) return; const int64_t child_var_ref = @@ -166,7 +164,7 @@ VariablesRequestHandler::Run(const VariablesArguments &arguments) const { } } - return VariablesResponseBody{variables}; + return VariablesResponseBody{std::move(variables)}; } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 1f9719110cedb..89678c4f6bc45 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -745,66 +745,62 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, return llvm::json::Value(std::move(event)); } -const char *GetNonNullVariableName(lldb::SBValue &v) { - const char *name = v.GetName(); - return name ? name : "<null>"; +llvm::StringRef GetNonNullVariableName(lldb::SBValue &v) { + const llvm::StringRef name = v.GetName(); + return !name.empty() ? name : "<null>"; } std::string CreateUniqueVariableNameForDisplay(lldb::SBValue &v, bool is_name_duplicated) { - lldb::SBStream name_builder; - name_builder.Print(GetNonNullVariableName(v)); + std::string unique_name{}; + llvm::raw_string_ostream name_builder(unique_name); + name_builder << GetNonNullVariableName(v); if (is_name_duplicated) { - lldb::SBDeclaration declaration = v.GetDeclaration(); - const char *file_name = declaration.GetFileSpec().GetFilename(); + const lldb::SBDeclaration declaration = v.GetDeclaration(); + const llvm::StringRef file_name = declaration.GetFileSpec().GetFilename(); const uint32_t line = declaration.GetLine(); - if (file_name != nullptr && line > 0) - name_builder.Printf(" @ %s:%u", file_name, line); - else if (const char *location = v.GetLocation()) - name_builder.Printf(" @ %s", location); + if (!file_name.empty() && line != 0 && line != LLDB_INVALID_LINE_NUMBER) + name_builder << llvm::formatv(" @ {}:{}", file_name, line); + else if (llvm::StringRef location = v.GetLocation(); !location.empty()) + name_builder << llvm::formatv(" @ {}", location); } - return name_builder.GetData(); -} - -VariableDescription::VariableDescription(lldb::SBValue v, - bool auto_variable_summaries, - bool format_hex, - bool is_name_duplicated, - std::optional<std::string> custom_name) - : v(v) { - name = custom_name - ? *custom_name - : CreateUniqueVariableNameForDisplay(v, is_name_duplicated); - - type_obj = v.GetType(); - std::string raw_display_type_name = - llvm::StringRef(type_obj.GetDisplayTypeName()).str(); + return unique_name; +} + +VariableDescription::VariableDescription( + lldb::SBValue val, bool auto_variable_summaries, bool format_hex, + bool is_name_duplicated, std::optional<llvm::StringRef> custom_name) + : val(val) { + name = custom_name.value_or( + CreateUniqueVariableNameForDisplay(val, is_name_duplicated)); + + type_obj = val.GetType(); + llvm::StringRef raw_display_type_name = type_obj.GetDisplayTypeName(); display_type_name = !raw_display_type_name.empty() ? raw_display_type_name : NO_TYPENAME; // Only format hex/default if there is no existing special format. - if (v.GetFormat() == lldb::eFormatDefault || - v.GetFormat() == lldb::eFormatHex) { - if (format_hex) - v.SetFormat(lldb::eFormatHex); - else - v.SetFormat(lldb::eFormatDefault); + if (const lldb::Format current_format = val.GetFormat(); + current_format == lldb::eFormatDefault || + current_format == lldb::eFormatHex) { + + val.SetFormat(format_hex ? lldb::eFormatHex : lldb::eFormatDefault); } llvm::raw_string_ostream os_display_value(display_value); - if (lldb::SBError sb_error = v.GetError(); sb_error.Fail()) { + if (lldb::SBError sb_error = val.GetError(); sb_error.Fail()) { error = sb_error.GetCString(); os_display_value << "<error: " << error << ">"; } else { - value = llvm::StringRef(v.GetValue()).str(); - summary = llvm::StringRef(v.GetSummary()).str(); + value = val.GetValue(); + summary = val.GetSummary(); if (summary.empty() && auto_variable_summaries) - auto_summary = TryCreateAutoSummary(v); + auto_summary = TryCreateAutoSummary(val); std::optional<std::string> effective_summary = - !summary.empty() ? summary : auto_summary; + !summary.empty() ? summary.str() : auto_summary; if (!value.empty()) { os_display_value << value; @@ -817,7 +813,7 @@ VariableDescription::VariableDescription(lldb::SBValue v, } else { if (!raw_display_type_name.empty()) { os_display_value << raw_display_type_name; - lldb::addr_t address = v.GetLoadAddress(); + lldb::addr_t address = val.GetLoadAddress(); if (address != LLDB_INVALID_ADDRESS) os_display_value << " @ " << llvm::format_hex(address, 0); } @@ -825,7 +821,7 @@ VariableDescription::VariableDescription(lldb::SBValue v, } lldb::SBStream evaluateStream; - v.GetExpressionPath(evaluateStream); + val.GetExpressionPath(evaluateStream); evaluate_name = llvm::StringRef(evaluateStream.GetData()).str(); } @@ -835,13 +831,13 @@ std::string VariableDescription::GetResult(protocol::EvaluateContext context) { if (context != protocol::eEvaluateContextRepl) return display_value; - if (!v.IsValid()) + if (!val.IsValid()) return display_value; // Try the SBValue::GetDescription(), which may call into language runtime // specific formatters (see ValueObjectPrinter). lldb::SBStream stream; - v.GetDescription(stream); + val.GetDescription(stream); llvm::StringRef description = stream.GetData(); return description.trim().str(); } diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 0a907599fc9ee..1e38de6f5b80d 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -316,7 +316,7 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, /// \return /// The variable name of \a value or a default placeholder. -const char *GetNonNullVariableName(lldb::SBValue &value); +llvm::StringRef GetNonNullVariableName(lldb::SBValue &value); /// VSCode can't display two variables with the same name, so we need to /// distinguish them by using a suffix. @@ -338,21 +338,21 @@ struct VariableDescription { // The variable path for this variable. std::string evaluate_name; // The output of SBValue.GetValue() if it doesn't fail. It might be empty. - std::string value; + llvm::StringRef value; // The summary string of this variable. It might be empty. - std::string summary; + llvm::StringRef summary; // The auto summary if using `enableAutoVariableSummaries`. std::optional<std::string> auto_summary; // The type of this variable. lldb::SBType type_obj; // The display type name of this variable. - std::string display_type_name; + llvm::StringRef display_type_name; /// The SBValue for this variable. - lldb::SBValue v; + lldb::SBValue val; VariableDescription(lldb::SBValue v, bool auto_variable_summaries, bool format_hex = false, bool is_name_duplicated = false, - std::optional<std::string> custom_name = {}); + std::optional<llvm::StringRef> custom_name = {}); /// Returns a description of the value appropriate for the specified context. std::string GetResult(protocol::EvaluateContext context); diff --git a/lldb/tools/lldb-dap/ProtocolUtils.cpp b/lldb/tools/lldb-dap/ProtocolUtils.cpp index acf31b03f7af0..af583fdef52fb 100644 --- a/lldb/tools/lldb-dap/ProtocolUtils.cpp +++ b/lldb/tools/lldb-dap/ProtocolUtils.cpp @@ -242,7 +242,7 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp) { Variable CreateVariable(lldb::SBValue v, int64_t var_ref, bool format_hex, bool auto_variable_summaries, bool synthetic_child_debugging, bool is_name_duplicated, - std::optional<std::string> custom_name) { + std::optional<llvm::StringRef> custom_name) { VariableDescription desc(v, auto_variable_summaries, format_hex, is_name_duplicated, custom_name); Variable var; diff --git a/lldb/tools/lldb-dap/ProtocolUtils.h b/lldb/tools/lldb-dap/ProtocolUtils.h index f4d576ba9f608..89839f2caad50 100644 --- a/lldb/tools/lldb-dap/ProtocolUtils.h +++ b/lldb/tools/lldb-dap/ProtocolUtils.h @@ -143,11 +143,11 @@ std::string ConvertDebugInfoSizeToString(uint64_t debug_size); /// /// \return /// A Variable representing the given value. -protocol::Variable CreateVariable(lldb::SBValue v, int64_t var_ref, - bool format_hex, bool auto_variable_summaries, - bool synthetic_child_debugging, - bool is_name_duplicated, - std::optional<std::string> custom_name = {}); +protocol::Variable +CreateVariable(lldb::SBValue v, int64_t var_ref, bool format_hex, + bool auto_variable_summaries, bool synthetic_child_debugging, + bool is_name_duplicated, + std::optional<llvm::StringRef> custom_name = {}); } // namespace lldb_dap `````````` </details> https://github.com/llvm/llvm-project/pull/172661 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
