jasonmolenda created this revision.
jasonmolenda added reviewers: jingham, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
Looking at debugserver's memory use a bit, I noticed that the SendPacket method
which takes a std::string& argument, is often called with a std::string object
that the caller calls c_str() on, so a temporary copy of the string is created.
This patch removes those c_str() method calls.
We also have a common pattern where we have/create a JSONGenerator::ObjectSP,
dump the string representation of the object into a ostringstream, and then
binary escape the ostringstream. We end up with the original ObjectSP
dictionary, the ostringstream print of it, and the escaped std::string copy of
it. Then we pass that to SendPacket which may create a compressed std::string
of it. This patch frees those objects as soon as they are no longer needed, so
our peak memory use is limited to a narrower window.
In the future I want to add a JSONGenerator::DumpEscaped() method that formats
its output following the binary escape protocol. Every packet that sends JSON
responses must escape it before sending, so there's no point in even generating
the un-escaped version in the first place. It forces us to always have two
copies of the string in heap for at least a little while.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D122848
Files:
lldb/tools/debugserver/source/RNBRemote.cpp
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===================================================================
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -584,6 +584,8 @@
stream << "JSON-async:";
dictionary.Dump(stream);
const std::string payload = binary_encode_string(stream.str());
+ stream.str(std::string());
+ stream.clear();
return SendPacket(payload);
}
@@ -3760,7 +3762,7 @@
return_message +=
cstring_to_asciihex_string("debugserver is x86_64 binary running in "
"translation, attached failed.");
- SendPacket(return_message.c_str());
+ SendPacket(return_message);
return rnb_err;
}
@@ -3853,14 +3855,14 @@
DNBLogError("Tried to attach to pid that doesn't exist");
std::string return_message = "E96;";
return_message += cstring_to_asciihex_string("no such process.");
- return SendPacket(return_message.c_str());
+ return SendPacket(return_message);
}
if (process_is_already_being_debugged (pid_attaching_to)) {
DNBLogError("Tried to attach to process already being debugged");
std::string return_message = "E96;";
return_message += cstring_to_asciihex_string("tried to attach to "
"process already being debugged");
- return SendPacket(return_message.c_str());
+ return SendPacket(return_message);
}
uid_t my_uid, process_uid;
if (attach_failed_due_to_uid_mismatch (pid_attaching_to,
@@ -3881,7 +3883,7 @@
+ my_username + "' and process is running "
"as user '" + process_username + "'";
return_message += cstring_to_asciihex_string(msg.c_str());
- return SendPacket(return_message.c_str());
+ return SendPacket(return_message);
}
if (!login_session_has_gui_access() && !developer_mode_enabled()) {
DNBLogError("Developer mode is not enabled and this is a "
@@ -3891,7 +3893,7 @@
"not enabled on this machine "
"and this is a non-interactive "
"debug session.");
- return SendPacket(return_message.c_str());
+ return SendPacket(return_message);
}
if (!login_session_has_gui_access()) {
DNBLogError("This is a non-interactive session");
@@ -3900,7 +3902,7 @@
"non-interactive debug session, "
"cannot get permission to debug "
"processes.");
- return SendPacket(return_message.c_str());
+ return SendPacket(return_message);
}
}
@@ -3923,7 +3925,7 @@
std::string default_return_msg = "E96;";
default_return_msg += cstring_to_asciihex_string
(error_explainer.c_str());
- SendPacket (default_return_msg.c_str());
+ SendPacket (default_return_msg);
DNBLogError("Attach failed: \"%s\".", err_str);
return rnb_err;
}
@@ -4347,7 +4349,7 @@
std::string data = DNBProcessGetProfileData(pid, scan_type);
if (!data.empty()) {
- return SendPacket(data.c_str());
+ return SendPacket(data);
} else {
return SendPacket("OK");
}
@@ -5557,9 +5559,12 @@
if (threads_info_sp) {
std::ostringstream strm;
threads_info_sp->Dump(strm);
+ threads_info_sp->Clear();
std::string binary_packet = binary_encode_string(strm.str());
+ strm.str(std::string());
+ strm.clear();
if (!binary_packet.empty())
- return SendPacket(binary_packet.c_str());
+ return SendPacket(binary_packet);
}
}
return SendPacket("E85");
@@ -5815,6 +5820,8 @@
json << "}";
std::string json_quoted = binary_encode_string(json.str());
+ json.str(std::string());
+ json.clear();
return SendPacket(json_quoted);
}
}
@@ -5881,9 +5888,12 @@
if (json_sp.get()) {
std::ostringstream json_str;
json_sp->Dump(json_str);
+ json_sp->Clear();
if (json_str.str().size() > 0) {
std::string json_str_quoted = binary_encode_string(json_str.str());
- return SendPacket(json_str_quoted.c_str());
+ json_str.str(std::string());
+ json_str.clear();
+ return SendPacket(json_str_quoted);
} else {
SendPacket("E84");
}
@@ -5914,9 +5924,12 @@
if (json_sp.get()) {
std::ostringstream json_str;
json_sp->Dump(json_str);
+ json_sp->Clear();
if (json_str.str().size() > 0) {
std::string json_str_quoted = binary_encode_string(json_str.str());
- return SendPacket(json_str_quoted.c_str());
+ json_str.str(std::string());
+ json_str.clear();
+ return SendPacket(json_str_quoted);
} else {
SendPacket("E86");
}
@@ -6116,7 +6129,7 @@
reply << "qSymbol:";
for (size_t i = 0; i < symbol_name.size(); ++i)
reply << RAWHEX8(symbol_name[i]);
- return SendPacket(reply.str().c_str());
+ return SendPacket(reply.str());
}
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
