[pushed] Reuse buffers across gdb_pretty_print_insn calls (Re: [PATCH v4 1/2] Add back gdb_pretty_print_insn)

[Pedro Alves <palves@redhat.com>]

On 02/02/2017 01:20 AM, Simon Marchi wrote:
>> -/* Prints the instruction INSN into UIOUT and returns the length of
>> -   the printed instruction in bytes.  */
>> +/* Class used to pretty-print an instruction.  */
> 
> I'd say "...to pretty-print instructions.", since the goal is to reuse
> it for multiple instructions.
> 

I've pushed in the v4 series now, and this follow up as well, as below.
(only changes compared to yesterday's are struct -> class, and making
the arch() method private.)

I somehow confused myself and pushed the patch without addressing your
comment above, but I've fixed it in a separate commit.

From 8b172ce7c9435095d14e0bd98cd431bb9584e95e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 2 Feb 2017 11:11:47 +0000
Subject: [PATCH] Reuse buffers across gdb_pretty_print_insn calls

gdb_pretty_print_insn allocates and destroys a couple local buffers
each time it is called, which can be many times when disassembling a
region of memory.  Avoid that overhead by adding a new class that
holds the buffers and making gdb_pretty_print_insn a method of that
class, so that the buffers can be reused across calls.

gdb/ChangeLog:
2017-02-02  Pedro Alves  <palves@redhat.com>

	* disasm.c (gdb_pretty_print_insn): Rename to ...
	(gdb_pretty_print_disassembler::pretty_print_insn): ... this.
	Remove gdbarch parameter.  Adapt to clear the object's buffers
	instead of allocating new buffers, and to print using the object's
	gdb_disassembler instead of calling gdb_print_insn.
	(dump_insns): Use gdb_pretty_print_disassembler.
	* disasm.h (gdb_pretty_print_insn): Delete declaration.
	(gdb_pretty_print_disassembler): New class.
	* record-btrace.c (btrace_insn_history): Use
	gdb_pretty_print_disassembler.
---
 gdb/ChangeLog       | 13 +++++++++++++
 gdb/disasm.c        | 25 ++++++++++++++-----------
 gdb/disasm.h        | 31 +++++++++++++++++++++++++++----
 gdb/record-btrace.c |  4 +++-
 4 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e3bca38..38a3beb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
 2017-02-02  Pedro Alves  <palves@redhat.com>
 
+	* disasm.c (gdb_pretty_print_insn): Rename to ...
+	(gdb_pretty_print_disassembler::pretty_print_insn): ... this.
+	Remove gdbarch parameter.  Adapt to clear the object's buffers
+	instead of allocating new buffers, and to print using the object's
+	gdb_disassembler instead of calling gdb_print_insn.
+	(dump_insns): Use gdb_pretty_print_disassembler.
+	* disasm.h (gdb_pretty_print_insn): Delete declaration.
+	(gdb_pretty_print_disassembler): New class.
+	* record-btrace.c (btrace_insn_history): Use
+	gdb_pretty_print_disassembler.
+
+2017-02-02  Pedro Alves  <palves@redhat.com>
+
 	* ada-lang.c (type_as_string): Use string_file.
 	* ada-valprint.c (ada_print_floating): Use string_file.
 	* ada-varobj.c (ada_varobj_scalar_image)
diff --git a/gdb/disasm.c b/gdb/disasm.c
index 5f0e86a..64d6684 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -182,9 +182,9 @@ compare_lines (const void *mle1p, const void *mle2p)
 /* See disasm.h.  */
 
 int
-gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
-		       const struct disasm_insn *insn,
-		       int flags)
+gdb_pretty_print_disassembler::pretty_print_insn (struct ui_out *uiout,
+						  const struct disasm_insn *insn,
+						  int flags)
 {
   /* parts of the symbolic representation of the address */
   int unmapped;
@@ -195,6 +195,7 @@ gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
   char *filename = NULL;
   char *name = NULL;
   CORE_ADDR pc;
+  struct gdbarch *gdbarch = arch ();
 
   ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
   pc = insn->addr;
@@ -248,7 +249,7 @@ gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
   if (name != NULL)
     xfree (name);
 
-  string_file stb;
+  m_insn_stb.clear ();
 
   if (flags & DISASSEMBLY_RAW_INSN)
     {
@@ -259,25 +260,25 @@ gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
 
       /* Build the opcodes using a temporary stream so we can
 	 write them out in a single go for the MI.  */
-      string_file opcode_stream;
+      m_opcode_stb.clear ();
 
-      size = gdb_print_insn (gdbarch, pc, &stb, NULL);
+      size = m_di.print_insn (pc);
       end_pc = pc + size;
 
       for (;pc < end_pc; ++pc)
 	{
 	  read_code (pc, &data, 1);
-	  opcode_stream.printf ("%s%02x", spacer, (unsigned) data);
+	  m_opcode_stb.printf ("%s%02x", spacer, (unsigned) data);
 	  spacer = " ";
 	}
 
-      uiout->field_stream ("opcodes", opcode_stream);
+      uiout->field_stream ("opcodes", m_opcode_stb);
       uiout->text ("\t");
     }
   else
-    size = gdb_print_insn (gdbarch, pc, &stb, NULL);
+    size = m_di.print_insn (pc);
 
-  uiout->field_stream ("inst", stb);
+  uiout->field_stream ("inst", m_insn_stb);
   do_cleanups (ui_out_chain);
   uiout->text ("\n");
 
@@ -295,11 +296,13 @@ dump_insns (struct gdbarch *gdbarch,
   memset (&insn, 0, sizeof (insn));
   insn.addr = low;
 
+  gdb_pretty_print_disassembler disasm (gdbarch);
+
   while (insn.addr < high && (how_many < 0 || num_displayed < how_many))
     {
       int size;
 
-      size = gdb_pretty_print_insn (gdbarch, uiout, &insn, flags);
+      size = disasm.pretty_print_insn (uiout, &insn, flags);
       if (size <= 0)
 	break;
 
diff --git a/gdb/disasm.h b/gdb/disasm.h
index dcc01e1..385cae6 100644
--- a/gdb/disasm.h
+++ b/gdb/disasm.h
@@ -97,11 +97,34 @@ extern void gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
 extern int gdb_print_insn (struct gdbarch *gdbarch, CORE_ADDR memaddr,
 			   struct ui_file *stream, int *branch_delay_insns);
 
-/* Prints the instruction INSN into UIOUT and returns the length of
-   the printed instruction in bytes.  */
+/* Class used to pretty-print an instruction.  */
 
-extern int gdb_pretty_print_insn (struct gdbarch *gdbarch, struct ui_out *uiout,
-				  const struct disasm_insn *insn, int flags);
+class gdb_pretty_print_disassembler
+{
+public:
+  explicit gdb_pretty_print_disassembler (struct gdbarch *gdbarch)
+    : m_di (gdbarch, &m_insn_stb)
+  {}
+
+  /* Prints the instruction INSN into UIOUT and returns the length of
+     the printed instruction in bytes.  */
+  int pretty_print_insn (struct ui_out *uiout, const struct disasm_insn *insn,
+			 int flags);
+
+private:
+  /* Returns the architecture used for disassembling.  */
+  struct gdbarch *arch () { return m_di.arch (); }
+
+  /* The disassembler used for instruction printing.  */
+  gdb_disassembler m_di;
+
+  /* The buffer used to build the instruction string.  The
+     disassembler is initialized with this stream.  */
+  string_file m_insn_stb;
+
+  /* The buffer used to build the raw opcodes string.  */
+  string_file m_opcode_stb;
+};
 
 /* Return the length in bytes of the instruction at address MEMADDR in
    debugged memory.  */
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 35e96ad..daa0696 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -719,6 +719,8 @@ btrace_insn_history (struct ui_out *uiout,
      instructions corresponding to that line.  */
   ui_item_chain = NULL;
 
+  gdb_pretty_print_disassembler disasm (gdbarch);
+
   for (it = *begin; btrace_insn_cmp (&it, end) != 0; btrace_insn_next (&it, 1))
     {
       const struct btrace_insn *insn;
@@ -772,7 +774,7 @@ btrace_insn_history (struct ui_out *uiout,
 	  if ((insn->flags & BTRACE_INSN_FLAG_SPECULATIVE) != 0)
 	    dinsn.is_speculative = 1;
 
-	  gdb_pretty_print_insn (gdbarch, uiout, &dinsn, flags);
+	  disasm.pretty_print_insn (uiout, &dinsn, flags);
 	}
     }
 
-- 
2.5.5