[Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

Fri Jun 7 09:38:07 PDT 2019

guiandrade added a comment.

In D62931#1532510 <https://reviews.llvm.org/D62931#1532510>, @clayborg wrote:

> In D62931#1531965 <https://reviews.llvm.org/D62931#1531965>, @labath wrote:
>
> >
>
>
> There are so many various GDB remote servers and it is hard to say what will work for most people. Might be worth testing this with the setting set to true and false on a complex program single step on mac, linux and android and verifying that stepping speeds don't regress at all. Android tends to have hundreds of threads which usually makes it the slowest of all when stepping. If we don't see regressions, then I am fine with no setting. Every scenario I know of benefits from fewer packets with larger content (macOS, watchOS, linux, android) so I am fine just enabling it. Maybe it would be good to check what GDB does by default as it might tell us, via comments, why they don't use it if they did run into issues.

How can we run those tests? Is there an automated way to do that?

> So I am ok with no setting as long as perf doesn't regress on complex steps and as long as GDB uses 'g' packets by default.

I think they first try to use 'g' packets - https://github.com/bminor/binutils-gdb/blob/4fa0265edea0940b865678d93749e224547dd36a/gdb/remote.c#L8130  (I'm not familiar with that code at all, so I may be wrong)

================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1722
+        thread_sp = std::make_shared<ThreadGDBRemote>(
+            *this, tid, GetGlobalPluginProperties()->GetPacketTimeout());
         LLDB_LOGV(log, "Making new thread: {0} for thread ID: {1:x}.",
----------------
note to self: fix wrong method call

================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1839
+        thread_sp = std::make_shared<ThreadGDBRemote>(
+            *this, tid, GetGlobalPluginProperties()->GetPacketTimeout());
         m_thread_list_real.AddThread(thread_sp);
----------------
note to self: fix wrong method call

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62931/new/

https://reviews.llvm.org/D62931