guiandrade created this revision.
guiandrade added a reviewer: labath.
guiandrade added projects: LLDB, LLVM.
Herald added a subscriber: lldb-commits.

Following up on https://reviews.llvm.org/D62221 
<https://reviews.llvm.org/D62221>, this change introduces
the setting plugin.process.gdb-remote.try-to-use-g-packet. When that is on
and the server supports the use of 'g' packets, those will be used regardless
of whether 'p' packets are supported.

Using 'g' packets can improve performance by reducing the number of packets
exchanged between client and server when a large number of registers
needs to be fetched.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D62931

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -128,6 +128,12 @@
   ASSERT_EQ(0,
             memcmp(buffer_sp->GetBytes(), one_register, sizeof one_register));
 
+  async_result = std::async(std::launch::async,
+                            [&] { return client.GetgPacketSupported(tid); });
+  Handle_QThreadSuffixSupported(server, true);
+  HandlePacket(server, "g;thread:0047;", all_registers_hex);
+  ASSERT_TRUE(async_result.get());
+
   read_result = std::async(std::launch::async,
                            [&] { return client.ReadAllRegisters(tid); });
   HandlePacket(server, "g;thread:0047;", all_registers_hex);
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -25,7 +25,7 @@
 
 class ThreadGDBRemote : public Thread {
 public:
-  ThreadGDBRemote(Process &process, lldb::tid_t tid);
+  ThreadGDBRemote(Process &process, lldb::tid_t tid, bool try_to_use_g_packet);
 
   ~ThreadGDBRemote() override;
 
@@ -100,6 +100,7 @@
   uint64_t
       m_queue_serial_number; // Queue info from stop reply/stop info for thread
   lldb_private::LazyBool m_associated_with_libdispatch_queue;
+  bool m_try_to_use_g_packet;
 
   bool PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef<uint8_t> data);
 
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -33,12 +33,14 @@
 
 // Thread Registers
 
-ThreadGDBRemote::ThreadGDBRemote(Process &process, lldb::tid_t tid)
+ThreadGDBRemote::ThreadGDBRemote(Process &process, lldb::tid_t tid,
+                                 bool try_to_use_g_packet)
     : Thread(process, tid), m_thread_name(), m_dispatch_queue_name(),
       m_thread_dispatch_qaddr(LLDB_INVALID_ADDRESS),
       m_dispatch_queue_t(LLDB_INVALID_ADDRESS), m_queue_kind(eQueueKindUnknown),
       m_queue_serial_number(LLDB_INVALID_QUEUE_ID),
-      m_associated_with_libdispatch_queue(eLazyBoolCalculate) {
+      m_associated_with_libdispatch_queue(eLazyBoolCalculate),
+      m_try_to_use_g_packet(try_to_use_g_packet) {
   Log *log(GetLogIfAnyCategoriesSet(GDBR_LOG_THREAD));
   LLDB_LOG(log, "this = {0}, pid = {1}, tid = {2}", this, process.GetID(),
            GetID());
@@ -303,9 +305,12 @@
     if (process_sp) {
       ProcessGDBRemote *gdb_process =
           static_cast<ProcessGDBRemote *>(process_sp.get());
-      // read_all_registers_at_once will be true if 'p' packet is not
-      // supported.
+      // read_all_registers_at_once will be true if either the flag
+      // m_try_to_use_g_packet is true and the server supports 'g' packet
+      // or if 'p' packet is not supported.
       bool read_all_registers_at_once =
+          (m_try_to_use_g_packet &&
+           gdb_process->GetGDBRemote().GetgPacketSupported(GetID())) ||
           !gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
       reg_ctx_sp = std::make_shared<GDBRemoteRegisterContext>(
           *this, concrete_frame_idx, gdb_process->m_register_info,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -120,9 +120,15 @@
      , nullptr, {},
      "Specify the default packet timeout in seconds."},
     {"target-definition-file", OptionValue::eTypeFileSpec, true, 0, nullptr, {},
-     "The file that provides the description for remote target registers."}};
-
-enum { ePropertyPacketTimeout, ePropertyTargetDefinitionFile };
+     "The file that provides the description for remote target registers."},
+    {"try-to-use-g-packet", OptionValue::eTypeBoolean, true, 0, nullptr, {},
+     "Specify if the server should try to use 'g' packets."}};
+
+enum {
+  ePropertyPacketTimeout,
+  ePropertyTargetDefinitionFile,
+  ePropertyTryToUseGPacket
+};
 
 class PluginProperties : public Properties {
 public:
@@ -152,6 +158,11 @@
     const uint32_t idx = ePropertyTargetDefinitionFile;
     return m_collection_sp->GetPropertyAtIndexAsFileSpec(nullptr, idx);
   }
+
+  bool GetTryToUseGPacket() const {
+    const uint32_t idx = ePropertyTryToUseGPacket;
+    return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, false);
+  }
 };
 
 typedef std::shared_ptr<PluginProperties> ProcessKDPPropertiesSP;
@@ -1707,7 +1718,8 @@
       ThreadSP thread_sp(
           old_thread_list_copy.RemoveThreadByProtocolID(tid, false));
       if (!thread_sp) {
-        thread_sp = std::make_shared<ThreadGDBRemote>(*this, tid);
+        thread_sp = std::make_shared<ThreadGDBRemote>(
+            *this, tid, GetGlobalPluginProperties()->GetPacketTimeout());
         LLDB_LOGV(log, "Making new thread: {0} for thread ID: {1:x}.",
                   thread_sp.get(), thread_sp->GetID());
       } else {
@@ -1823,7 +1835,8 @@
 
       if (!thread_sp) {
         // Create the thread if we need to
-        thread_sp = std::make_shared<ThreadGDBRemote>(*this, tid);
+        thread_sp = std::make_shared<ThreadGDBRemote>(
+            *this, tid, GetGlobalPluginProperties()->GetPacketTimeout());
         m_thread_list_real.AddThread(thread_sp);
       }
     }
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -234,6 +234,8 @@
 
   bool GetpPacketSupported(lldb::tid_t tid);
 
+  bool GetgPacketSupported(lldb::tid_t tid);
+
   bool GetxPacketSupported();
 
   bool GetVAttachOrWaitSupported();
@@ -514,6 +516,7 @@
   LazyBool m_prepare_for_reg_writing_reply;
   LazyBool m_supports_p;
   LazyBool m_supports_x;
+  LazyBool m_supports_g;
   LazyBool m_avoid_g_packets;
   LazyBool m_supports_QSaveRegisterState;
   LazyBool m_supports_qXfer_auxv_read;
@@ -592,6 +595,8 @@
   Status GetQXferMemoryMapRegionInfo(lldb::addr_t addr,
                                      MemoryRegionInfo &region);
 
+  LazyBool GetThreadPacketSupported(lldb::tid_t tid, llvm::StringRef packetStr);
+
 private:
   DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunicationClient);
 };
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -72,7 +72,7 @@
       m_attach_or_wait_reply(eLazyBoolCalculate),
       m_prepare_for_reg_writing_reply(eLazyBoolCalculate),
       m_supports_p(eLazyBoolCalculate), m_supports_x(eLazyBoolCalculate),
-      m_avoid_g_packets(eLazyBoolCalculate),
+      m_supports_g(eLazyBoolCalculate), m_avoid_g_packets(eLazyBoolCalculate),
       m_supports_QSaveRegisterState(eLazyBoolCalculate),
       m_supports_qXfer_auxv_read(eLazyBoolCalculate),
       m_supports_qXfer_libraries_read(eLazyBoolCalculate),
@@ -546,21 +546,30 @@
 //
 // Takes a valid thread ID because p needs to apply to a thread.
 bool GDBRemoteCommunicationClient::GetpPacketSupported(lldb::tid_t tid) {
-  if (m_supports_p == eLazyBoolCalculate) {
-    m_supports_p = eLazyBoolNo;
-    StreamString payload;
-    payload.PutCString("p0");
-    StringExtractorGDBRemote response;
-    if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload),
-                                                   response, false) ==
-            PacketResult::Success &&
-        response.IsNormalResponse()) {
-      m_supports_p = eLazyBoolYes;
-    }
-  }
+  if (m_supports_p == eLazyBoolCalculate)
+    m_supports_p = GetThreadPacketSupported(tid, "p0");
   return m_supports_p;
 }
 
+bool GDBRemoteCommunicationClient::GetgPacketSupported(lldb::tid_t tid) {
+  if (m_supports_g == eLazyBoolCalculate)
+    m_supports_g = GetThreadPacketSupported(tid, "g");
+  return m_supports_g;
+}
+
+LazyBool GDBRemoteCommunicationClient::GetThreadPacketSupported(
+    lldb::tid_t tid, llvm::StringRef packetStr) {
+  StreamString payload;
+  payload.PutCString(packetStr);
+  StringExtractorGDBRemote response;
+  if (SendThreadSpecificPacketAndWaitForResponse(
+          tid, std::move(payload), response, false) == PacketResult::Success &&
+      response.IsNormalResponse()) {
+    return eLazyBoolYes;
+  }
+  return eLazyBoolNo;
+}
+
 StructuredData::ObjectSP GDBRemoteCommunicationClient::GetThreadsInfo() {
   // Get information on all threads at one using the "jThreadsInfo" packet
   StructuredData::ObjectSP object_sp;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to