jasonmolenda created this revision.
jasonmolenda added reviewers: JDevlieghere, jingham.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
Most packets which return JSON use JSONGenerator::Dump to print the string
representation of the data, and then pass that string to another method to
escape the #, $, }, and * characters which have special meaning in the gdb
remote serial protocol. This generate-then-escape two step process involves
duplicating the entire string, and for large data, can result in a lot of
memory use.
This patch adds JSONGenerator::DumpBinaryEscaped methods for those cases where
this is needed. The Dictionary, Array, and String methods involve duplicating
the existing Dump methods, mostly to call the corresponding DumpBinaryEscaped
methods on their children. The other JSON types do not need escaping, and call
the regular Dump methods.
This is the follow on patch to https://reviews.llvm.org/D122848 , the two of
them get us down from 3 string copies being held in heap, plus the JSON object
tree, to the JSON object tree and one copy. For a large list of shared
libraries, it can add up to quite a bit of memory use, and debugserver should
try to be low impact on the system.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D122882
Files:
lldb/tools/debugserver/source/JSONGenerator.h
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
@@ -582,9 +582,8 @@
// of these coming out at the wrong time (i.e. when the remote side
// is not waiting for a process control completion response).
stream << "JSON-async:";
- dictionary.Dump(stream);
- const std::string payload = binary_encode_string(stream.str());
- return SendPacket(payload);
+ dictionary.DumpBinaryEscaped(stream);
+ return SendPacket(stream.str());
}
// Given a std::string packet contents to send, possibly encode/compress it.
@@ -2793,6 +2792,7 @@
ostrm << std::hex << "jstopinfo:";
std::ostringstream json_strm;
threads_info_sp->Dump(json_strm);
+ threads_info_sp->Clear();
append_hexified_string(ostrm, json_strm.str());
ostrm << ';';
}
@@ -5556,11 +5556,10 @@
if (threads_info_sp) {
std::ostringstream strm;
- threads_info_sp->Dump(strm);
+ threads_info_sp->DumpBinaryEscaped(strm);
threads_info_sp->Clear();
- std::string binary_packet = binary_encode_string(strm.str());
- if (!binary_packet.empty())
- return SendPacket(binary_packet);
+ if (strm.str().size() > 0)
+ return SendPacket(strm.str());
}
}
return SendPacket("E85");
@@ -5881,11 +5880,10 @@
if (json_sp.get()) {
std::ostringstream json_str;
- json_sp->Dump(json_str);
+ json_sp->DumpBinaryEscaped(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);
+ return SendPacket(json_str.str());
} else {
SendPacket("E84");
}
@@ -5915,11 +5913,10 @@
if (json_sp.get()) {
std::ostringstream json_str;
- json_sp->Dump(json_str);
+ json_sp->DumpBinaryEscaped(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);
+ return SendPacket(json_str.str());
} else {
SendPacket("E86");
}
Index: lldb/tools/debugserver/source/JSONGenerator.h
===================================================================
--- lldb/tools/debugserver/source/JSONGenerator.h
+++ lldb/tools/debugserver/source/JSONGenerator.h
@@ -113,6 +113,8 @@
virtual void Dump(std::ostream &s) const = 0;
+ virtual void DumpBinaryEscaped(std::ostream &s) const = 0;
+
private:
Type m_type;
};
@@ -136,6 +138,17 @@
s << "]";
}
+ void DumpBinaryEscaped(std::ostream &s) const override {
+ s << "[";
+ const size_t arrsize = m_items.size();
+ for (size_t i = 0; i < arrsize; ++i) {
+ m_items[i]->DumpBinaryEscaped(s);
+ if (i + 1 < arrsize)
+ s << ",";
+ }
+ s << "]";
+ }
+
protected:
typedef std::vector<ObjectSP> collection;
collection m_items;
@@ -151,6 +164,8 @@
void Dump(std::ostream &s) const override { s << m_value; }
+ void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
protected:
uint64_t m_value;
};
@@ -165,6 +180,8 @@
void Dump(std::ostream &s) const override { s << m_value; }
+ void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
protected:
double m_value;
};
@@ -184,6 +201,8 @@
s << "false";
}
+ void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
protected:
bool m_value;
};
@@ -210,6 +229,24 @@
s << '"' << quoted.c_str() << '"';
}
+ void DumpBinaryEscaped(std::ostream &s) const override {
+ std::string quoted;
+ const size_t strsize = m_value.size();
+ for (size_t i = 0; i < strsize; ++i) {
+ char ch = m_value[i];
+ if (ch == '"')
+ quoted.push_back('\\');
+ // gdb remote serial protocol binary escaping
+ if (ch == '#' || ch == '$' || ch == '}' || ch == '*') {
+ quoted.push_back('}'); // 0x7d
+ quoted.push_back(ch ^ 0x20);
+ } else {
+ quoted.push_back(ch);
+ }
+ }
+ s << '"' << quoted.c_str() << '"';
+ }
+
protected:
std::string m_value;
};
@@ -269,7 +306,43 @@
s << "}";
}
+ void DumpBinaryEscaped(std::ostream &s) const override {
+ bool have_printed_one_elem = false;
+ s << "{";
+ for (collection::const_iterator iter = m_dict.begin();
+ iter != m_dict.end(); ++iter) {
+ if (!have_printed_one_elem) {
+ have_printed_one_elem = true;
+ } else {
+ s << ",";
+ }
+ s << "\"" << binary_encode_string(iter->first) << "\":";
+ iter->second->DumpBinaryEscaped(s);
+ }
+ // '}' must be escaped for the gdb remote serial
+ // protocol.
+ s << "}";
+ s << static_cast<char>('}' ^ 0x20);
+ }
+
protected:
+ std::string binary_encode_string(const std::string &s) const {
+ std::string output;
+ const size_t s_size = s.size();
+ const char *s_chars = s.c_str();
+
+ for (size_t i = 0; i < s_size; i++) {
+ unsigned char ch = *(s_chars + i);
+ if (ch == '#' || ch == '$' || ch == '}' || ch == '*') {
+ output.push_back('}'); // 0x7d
+ output.push_back(ch ^ 0x20);
+ } else {
+ output.push_back(ch);
+ }
+ }
+ return output;
+ }
+
// Keep the dictionary as a vector so the dictionary doesn't reorder itself
// when you dump it
// We aren't accessing keys by name, so this won't affect performance
@@ -288,6 +361,8 @@
void Dump(std::ostream &s) const override { s << "null"; }
+ void DumpBinaryEscaped(std::ostream &s) const override { Dump(s); }
+
protected:
};
@@ -304,6 +379,8 @@
void Dump(std::ostream &s) const override;
+ void DumpBinaryEscaped(std::ostream &s) const override;
+
private:
void *m_object;
};
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
