[Lldb-commits] [lldb] [lldb] [NFC] Remove min pkt size for compression setting (PR #81075)
https://github.com/JDevlieghere commented: 拾 https://github.com/llvm/llvm-project/pull/81075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Remove min pkt size for compression setting (PR #81075)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/81075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Remove min pkt size for compression setting (PR #81075)
jasonmolenda wrote: 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. https://github.com/llvm/llvm-project/pull/81075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Remove min pkt size for compression setting (PR #81075)
https://github.com/bulbazord approved this pull request. LGTM. I also noticed there's no tests for this functionality... https://github.com/llvm/llvm-project/pull/81075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Remove min pkt size for compression setting (PR #81075)
llvmbot wrote:
@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
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:;minsize:;
+// QEnableCompression: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");
}
``
https://github.com/llvm/llvm-project/pull/81075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Remove min pkt size for compression setting (PR #81075)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/81075
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.
>From 084a2ac7131d88fb07d9b9a66314c86717d2671d Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Wed, 7 Feb 2024 17:40:14 -0800
Subject: [PATCH] [lldb] [NFC] Remove min pkt size for compression setting
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.
---
lldb/docs/lldb-gdb-remote.txt | 8 ++-
lldb/tools/debugserver/source/RNBRemote.cpp | 25 +
2 files changed, 3 insertions(+), 30 deletions(-)
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:;minsize:;
+// QEnableCompression: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");
}
