public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 1/2] remote & target_extra_thread_info, use cache w/ qThreadExtraInfo and qP too
Date: Thu, 14 Jun 2018 16:28:00 -0000	[thread overview]
Message-ID: <20180614162811.31319-2-palves@redhat.com> (raw)
In-Reply-To: <20180614162811.31319-1-palves@redhat.com>

The following patch will make "info threads" call target_extra_thread_info
more frequently.  When I looked at the remote implementation, I noticed
that if we're not using qXfer:threads:read, then we'd be increasing the
remote protocol traffic.  This commit prevents that from happening.

Also, it removes a gratuitous local static buffer, which seems good on
its own.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* remote.c (remote_target::extra_thread_info): Delete
	'display_buf' and 'n' locals.  from the cache, regardless of
	packet mechanims is in use.  Use cache for qThreadExtra and qP
	methods too.
---
 gdb/remote.c | 51 +++++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 79ae8f66c7..400c75b661 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3826,12 +3826,9 @@ const char *
 remote_target::extra_thread_info (thread_info *tp)
 {
   struct remote_state *rs = get_remote_state ();
-  int result;
   int set;
   threadref id;
   struct gdb_ext_thread_info threadinfo;
-  static char display_buf[100];	/* arbitrary...  */
-  int n = 0;                    /* position in display_buf */
 
   if (rs->remote_desc == 0)		/* paranoia */
     internal_error (__FILE__, __LINE__,
@@ -3843,17 +3840,18 @@ remote_target::extra_thread_info (thread_info *tp)
        server doesn't know about it.  */
     return NULL;
 
+  std::string &extra = get_remote_thread_info (tp)->extra;
+
+  /* If already have cached info, use it.  */
+  if (!extra.empty ())
+    return extra.c_str ();
+
   if (packet_support (PACKET_qXfer_threads) == PACKET_ENABLE)
     {
-      struct thread_info *info = find_thread_ptid (tp->ptid);
-
-      if (info != NULL && info->priv != NULL)
-	{
-	  const std::string &extra = get_remote_thread_info (info)->extra;
-	  return !extra.empty () ? extra.c_str () : NULL;
-	}
-      else
-	return NULL;
+      /* If we're using qXfer:threads:read, then the extra info is
+	 included in the XML.  So if we didn't have anything cached,
+	 it's because there's really no extra info.  */
+      return NULL;
     }
 
   if (rs->use_threadextra_query)
@@ -3869,10 +3867,9 @@ remote_target::extra_thread_info (thread_info *tp)
       getpkt (&rs->buf, &rs->buf_size, 0);
       if (rs->buf[0] != 0)
 	{
-	  n = std::min (strlen (rs->buf) / 2, sizeof (display_buf));
-	  result = hex2bin (rs->buf, (gdb_byte *) display_buf, n);
-	  display_buf [result] = '\0';
-	  return display_buf;
+	  extra.resize (strlen (rs->buf) / 2);
+	  hex2bin (rs->buf, (gdb_byte *) &extra[0], extra.size ());
+	  return extra.c_str ();
 	}
     }
 
@@ -3885,22 +3882,20 @@ remote_target::extra_thread_info (thread_info *tp)
     if (threadinfo.active)
       {
 	if (*threadinfo.shortname)
-	  n += xsnprintf (&display_buf[0], sizeof (display_buf) - n,
-			  " Name: %s,", threadinfo.shortname);
+	  string_appendf (extra, " Name: %s", threadinfo.shortname);
 	if (*threadinfo.display)
-	  n += xsnprintf (&display_buf[n], sizeof (display_buf) - n,
-			  " State: %s,", threadinfo.display);
+	  {
+	    if (!extra.empty ())
+	      extra += ',';
+	    string_appendf (extra, " State: %s", threadinfo.display);
+	  }
 	if (*threadinfo.more_display)
-	  n += xsnprintf (&display_buf[n], sizeof (display_buf) - n,
-			  " Priority: %s", threadinfo.more_display);
-
-	if (n > 0)
 	  {
-	    /* For purely cosmetic reasons, clear up trailing commas.  */
-	    if (',' == display_buf[n-1])
-	      display_buf[n-1] = ' ';
-	    return display_buf;
+	    if (!extra.empty ())
+	      extra += ',';
+	    string_appendf (extra, " Priority: %s", threadinfo.more_display);
 	  }
+	return extra.c_str ();
       }
   return NULL;
 }
-- 
2.14.3

      parent reply	other threads:[~2018-06-14 16:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-14 16:28 [PATCH 0/2] Improve alignment of "info threads" output, align "Target Id" column Pedro Alves
2018-06-14 16:28 ` [PATCH 2/2] " Pedro Alves
2018-06-29 19:56   ` Pedro Alves
2018-06-14 16:28 ` Pedro Alves [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180614162811.31319-2-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).