[lldb] [NFC] Remove min pkt size for compression setting

[jasonmolenda]

debugserver will not compress small packets; the overhead of the compression header makes this useful for larger packets. I default to 384 bytes in debugserver, and added an option for lldb to request a different cutoff. This option has never been used in lldb in the past nine years, so I don't think there's any point to keeping it around.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)

Changes

debugserver will not compress small packets; the overhead of the compression header makes this useful for larger packets. I default to 384 bytes in debugserver, and added an option for lldb to request a different cutoff. This option has never been used in lldb in the past nine years, so I don't think there's any point to keeping it around.

---

Full diff: https://github.com/llvm/llvm-project/pull/81075.diff

2 Files Affected:

• (modified) lldb/docs/lldb-gdb-remote.txt (+2-6)
• (modified) lldb/tools/debugserver/source/RNBRemote.cpp (+1-24)

diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index 76ac3f28d73b0..6c29de61daba7 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -1998,16 +1998,12 @@ for this region.
 //  If the debug stub can support compression, it indictes this in the reply of the
 //  "qSupported" packet.  e.g.
 //   LLDB SENDS:    qSupported:xmlRegisters=i386,arm,mips
-//   STUB REPLIES:  qXfer:features:read+;SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;DefaultCompressionMinSize=384
+//   STUB REPLIES:  qXfer:features:read+;SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;
 //
 //  If lldb knows how to use any of these compression algorithms, it can ask that this
-//  compression mode be enabled.  It may optionally change the minimum packet size
-//  where compression is used.  Typically small packets do not benefit from compression,
-//  as well as compression headers -- compression is most beneficial with larger packets.
+//  compression mode be enabled.  
 //
 //  QEnableCompression:type:zlib-deflate;
-//  or
-//  QEnableCompression:type:zlib-deflate;minsize:512;
 //
 //  The debug stub should reply with an uncompressed "OK" packet to indicate that the
 //  request was accepted.  All further packets the stub sends will use this compression.
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index 20384920823d2..feea4c914ec53 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3575,8 +3575,6 @@ rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) {
 
   if (enable_compression) {
     reply << "SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;";
-    reply << "DefaultCompressionMinSize=" << std::dec << m_compression_minsize
-          << ";";
   }
 
 #if (defined(__arm64__) || defined(__aarch64__))
@@ -4482,46 +4480,25 @@ rnb_err_t RNBRemote::HandlePacket_SetEnableAsyncProfiling(const char *p) {
   return SendPacket("OK");
 }
 
-// QEnableCompression:type:<COMPRESSION-TYPE>;minsize:<MINIMUM PACKET SIZE TO
-// COMPRESS>;
+// QEnableCompression:type:<COMPRESSION-TYPE>;
 //
 // type: must be a type previously reported by the qXfer:features:
 // SupportedCompressions list
-//
-// minsize: is optional; by default the qXfer:features:
-// DefaultCompressionMinSize value is used
-// debugserver may have a better idea of what a good minimum packet size to
-// compress is than lldb.
 
 rnb_err_t RNBRemote::HandlePacket_QEnableCompression(const char *p) {
   p += sizeof("QEnableCompression:") - 1;
 
-  size_t new_compression_minsize = m_compression_minsize;
-  const char *new_compression_minsize_str = strstr(p, "minsize:");
-  if (new_compression_minsize_str) {
-    new_compression_minsize_str += strlen("minsize:");
-    errno = 0;
-    new_compression_minsize = strtoul(new_compression_minsize_str, NULL, 10);
-    if (errno != 0 || new_compression_minsize == ULONG_MAX) {
-      new_compression_minsize = m_compression_minsize;
-    }
-  }
-
   if (strstr(p, "type:zlib-deflate;") != nullptr) {
     EnableCompressionNextSendPacket(compression_types::zlib_deflate);
-    m_compression_minsize = new_compression_minsize;
     return SendPacket("OK");
   } else if (strstr(p, "type:lz4;") != nullptr) {
     EnableCompressionNextSendPacket(compression_types::lz4);
-    m_compression_minsize = new_compression_minsize;
     return SendPacket("OK");
   } else if (strstr(p, "type:lzma;") != nullptr) {
     EnableCompressionNextSendPacket(compression_types::lzma);
-    m_compression_minsize = new_compression_minsize;
     return SendPacket("OK");
   } else if (strstr(p, "type:lzfse;") != nullptr) {
     EnableCompressionNextSendPacket(compression_types::lzfse);
-    m_compression_minsize = new_compression_minsize;
     return SendPacket("OK");
   }
 

[bulbazord]

LGTM. I also noticed there's no tests for this functionality...

[jasonmolenda]

Thanks for the review. Yeah, testing isn't simple today because debugserver only enables compression on always-remote systems (iOS, watchOS, etc) (which may even be communicating over Bluetooth/Wifi), it doesn't enable this on macOS. We could have a mode to enable it on macOS for testing instead of a compile-time check like now. Now that mac-to-mac debugging is more common, this compile-time choice of when to enable it may need revisiting.

[JDevlieghere]

🥳