Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [rfa/i386] Make codestream deprecated?
@ 2002-12-01 16:39 Andrew Cagney
  2002-12-01 17:02 ` Daniel Jacobowitz
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Andrew Cagney @ 2002-12-01 16:39 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

Hello,

The attached is to make it clear that codestream isn't the way to go 
when trying to improve GDB's performance.  I don't see any urgency in 
actually removing the code, though.

The codestream has been supperseeded by a dcache.

thoughts?

ok to commit?

Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 11882 bytes --]

2002-12-01  Andrew Cagney  <ac131313@redhat.com>

	* i386-tdep.c: Replace `codestream' with `deprecated_codestream'.

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.100
diff -u -r1.100 i386-tdep.c
--- i386-tdep.c	28 Nov 2002 18:15:58 -0000	1.100
+++ i386-tdep.c	2 Dec 2002 00:17:08 -0000
@@ -214,55 +214,55 @@
    It probably needs the bfd library's help and support.  For now, the
    buffer size is set to 1.  (FIXME -fnf) */
 
-#define CODESTREAM_BUFSIZ 1	/* Was sizeof(int), see note above.  */
-static CORE_ADDR codestream_next_addr;
-static CORE_ADDR codestream_addr;
-static unsigned char codestream_buf[CODESTREAM_BUFSIZ];
-static int codestream_off;
-static int codestream_cnt;
-
-#define codestream_tell() (codestream_addr + codestream_off)
-#define codestream_peek() \
-  (codestream_cnt == 0 ? \
-   codestream_fill(1) : codestream_buf[codestream_off])
-#define codestream_get() \
-  (codestream_cnt-- == 0 ? \
-   codestream_fill(0) : codestream_buf[codestream_off++])
+#define DEPRECATED_CODESTREAM_BUFSIZ 1	/* Was sizeof(int), see note above.  */
+static CORE_ADDR deprecated_codestream_next_addr;
+static CORE_ADDR deprecated_codestream_addr;
+static unsigned char deprecated_codestream_buf[DEPRECATED_CODESTREAM_BUFSIZ];
+static int deprecated_codestream_off;
+static int deprecated_codestream_cnt;
+
+#define deprecated_codestream_tell() (deprecated_codestream_addr + deprecated_codestream_off)
+#define deprecated_codestream_peek() \
+  (deprecated_codestream_cnt == 0 ? \
+   deprecated_codestream_fill(1) : deprecated_codestream_buf[deprecated_codestream_off])
+#define deprecated_codestream_get() \
+  (deprecated_codestream_cnt-- == 0 ? \
+   deprecated_codestream_fill(0) : deprecated_codestream_buf[deprecated_codestream_off++])
 
 static unsigned char
-codestream_fill (int peek_flag)
+deprecated_codestream_fill (int peek_flag)
 {
-  codestream_addr = codestream_next_addr;
-  codestream_next_addr += CODESTREAM_BUFSIZ;
-  codestream_off = 0;
-  codestream_cnt = CODESTREAM_BUFSIZ;
-  read_memory (codestream_addr, (char *) codestream_buf, CODESTREAM_BUFSIZ);
+  deprecated_codestream_addr = deprecated_codestream_next_addr;
+  deprecated_codestream_next_addr += DEPRECATED_CODESTREAM_BUFSIZ;
+  deprecated_codestream_off = 0;
+  deprecated_codestream_cnt = DEPRECATED_CODESTREAM_BUFSIZ;
+  read_memory (deprecated_codestream_addr, (char *) deprecated_codestream_buf, DEPRECATED_CODESTREAM_BUFSIZ);
 
   if (peek_flag)
-    return (codestream_peek ());
+    return (deprecated_codestream_peek ());
   else
-    return (codestream_get ());
+    return (deprecated_codestream_get ());
 }
 
 static void
-codestream_seek (CORE_ADDR place)
+deprecated_codestream_seek (CORE_ADDR place)
 {
-  codestream_next_addr = place / CODESTREAM_BUFSIZ;
-  codestream_next_addr *= CODESTREAM_BUFSIZ;
-  codestream_cnt = 0;
-  codestream_fill (1);
-  while (codestream_tell () != place)
-    codestream_get ();
+  deprecated_codestream_next_addr = place / DEPRECATED_CODESTREAM_BUFSIZ;
+  deprecated_codestream_next_addr *= DEPRECATED_CODESTREAM_BUFSIZ;
+  deprecated_codestream_cnt = 0;
+  deprecated_codestream_fill (1);
+  while (deprecated_codestream_tell () != place)
+    deprecated_codestream_get ();
 }
 
 static void
-codestream_read (unsigned char *buf, int count)
+deprecated_codestream_read (unsigned char *buf, int count)
 {
   unsigned char *p;
   int i;
   p = buf;
   for (i = 0; i < count; i++)
-    *p++ = codestream_get ();
+    *p++ = deprecated_codestream_get ();
 }
 \f
 
@@ -277,22 +277,22 @@
   int data16;
   CORE_ADDR pos;
 
-  pos = codestream_tell ();
+  pos = deprecated_codestream_tell ();
 
   data16 = 0;
-  if (codestream_peek () == 0x66)
+  if (deprecated_codestream_peek () == 0x66)
     {
-      codestream_get ();
+      deprecated_codestream_get ();
       data16 = 1;
     }
 
-  switch (codestream_get ())
+  switch (deprecated_codestream_get ())
     {
     case 0xe9:
       /* Relative jump: if data16 == 0, disp32, else disp16.  */
       if (data16)
 	{
-	  codestream_read (buf, 2);
+	  deprecated_codestream_read (buf, 2);
 	  delta = extract_signed_integer (buf, 2);
 
 	  /* Include the size of the jmp instruction (including the
@@ -301,7 +301,7 @@
 	}
       else
 	{
-	  codestream_read (buf, 4);
+	  deprecated_codestream_read (buf, 4);
 	  delta = extract_signed_integer (buf, 4);
 
 	  pos += delta + 5;
@@ -309,32 +309,32 @@
       break;
     case 0xeb:
       /* Relative jump, disp8 (ignore data16).  */
-      codestream_read (buf, 1);
+      deprecated_codestream_read (buf, 1);
       /* Sign-extend it.  */
       delta = extract_signed_integer (buf, 1);
 
       pos += delta + 2;
       break;
     }
-  codestream_seek (pos);
+  deprecated_codestream_seek (pos);
 }
 
 /* Find & return the amount a local space allocated, and advance the
-   codestream to the first register push (if any).
+   deprecated_codestream to the first register push (if any).
 
    If the entry sequence doesn't make sense, return -1, and leave
-   codestream pointer at a random spot.  */
+   deprecated_codestream pointer at a random spot.  */
 
 static long
 i386_get_frame_setup (CORE_ADDR pc)
 {
   unsigned char op;
 
-  codestream_seek (pc);
+  deprecated_codestream_seek (pc);
 
   i386_follow_jump ();
 
-  op = codestream_get ();
+  op = deprecated_codestream_get ();
 
   if (op == 0x58)		/* popl %eax */
     {
@@ -354,15 +354,15 @@
       static unsigned char proto1[3] = { 0x87, 0x04, 0x24 };
       static unsigned char proto2[4] = { 0x87, 0x44, 0x24, 0x00 };
 
-      pos = codestream_tell ();
-      codestream_read (buf, 4);
+      pos = deprecated_codestream_tell ();
+      deprecated_codestream_read (buf, 4);
       if (memcmp (buf, proto1, 3) == 0)
 	pos += 3;
       else if (memcmp (buf, proto2, 4) == 0)
 	pos += 4;
 
-      codestream_seek (pos);
-      op = codestream_get ();	/* Update next opcode.  */
+      deprecated_codestream_seek (pos);
+      op = deprecated_codestream_get ();	/* Update next opcode.  */
     }
 
   if (op == 0x68 || op == 0x6a)
@@ -383,33 +383,33 @@
 
       /* Skip past the `pushl' instruction; it has either a one-byte 
          or a four-byte operand, depending on the opcode.  */
-      pos = codestream_tell ();
+      pos = deprecated_codestream_tell ();
       if (op == 0x68)
 	pos += 4;
       else
 	pos += 1;
-      codestream_seek (pos);
+      deprecated_codestream_seek (pos);
 
       /* Read the following 8 bytes, which should be "call _probe" (6
          bytes) followed by "addl $4,%esp" (2 bytes).  */
-      codestream_read (buf, sizeof (buf));
+      deprecated_codestream_read (buf, sizeof (buf));
       if (buf[0] == 0xe8 && buf[6] == 0xc4 && buf[7] == 0x4)
 	pos += sizeof (buf);
-      codestream_seek (pos);
-      op = codestream_get ();	/* Update next opcode.  */
+      deprecated_codestream_seek (pos);
+      op = deprecated_codestream_get ();	/* Update next opcode.  */
     }
 
   if (op == 0x55)		/* pushl %ebp */
     {
       /* Check for "movl %esp, %ebp" -- can be written in two ways.  */
-      switch (codestream_get ())
+      switch (deprecated_codestream_get ())
 	{
 	case 0x8b:
-	  if (codestream_get () != 0xec)
+	  if (deprecated_codestream_get () != 0xec)
 	    return -1;
 	  break;
 	case 0x89:
-	  if (codestream_get () != 0xe5)
+	  if (deprecated_codestream_get () != 0xe5)
 	    return -1;
 	  break;
 	default:
@@ -421,34 +421,34 @@
 
 	 NOTE: You can't subtract a 16 bit immediate from a 32 bit
 	 reg, so we don't have to worry about a data16 prefix.  */
-      op = codestream_peek ();
+      op = deprecated_codestream_peek ();
       if (op == 0x83)
 	{
 	  /* `subl' with 8 bit immediate.  */
-	  codestream_get ();
-	  if (codestream_get () != 0xec)
+	  deprecated_codestream_get ();
+	  if (deprecated_codestream_get () != 0xec)
 	    /* Some instruction starting with 0x83 other than `subl'.  */
 	    {
-	      codestream_seek (codestream_tell () - 2);
+	      deprecated_codestream_seek (deprecated_codestream_tell () - 2);
 	      return 0;
 	    }
 	  /* `subl' with signed byte immediate (though it wouldn't
 	     make sense to be negative).  */
-	  return (codestream_get ());
+	  return (deprecated_codestream_get ());
 	}
       else if (op == 0x81)
 	{
 	  char buf[4];
 	  /* Maybe it is `subl' with a 32 bit immedediate.  */
-	  codestream_get ();
-	  if (codestream_get () != 0xec)
+	  deprecated_codestream_get ();
+	  if (deprecated_codestream_get () != 0xec)
 	    /* Some instruction starting with 0x81 other than `subl'.  */
 	    {
-	      codestream_seek (codestream_tell () - 2);
+	      deprecated_codestream_seek (deprecated_codestream_tell () - 2);
 	      return 0;
 	    }
 	  /* It is `subl' with a 32 bit immediate.  */
-	  codestream_read ((unsigned char *) buf, 4);
+	  deprecated_codestream_read ((unsigned char *) buf, 4);
 	  return extract_signed_integer (buf, 4);
 	}
       else
@@ -460,8 +460,8 @@
     {
       char buf[2];
       /* `enter' with 16 bit unsigned immediate.  */
-      codestream_read ((unsigned char *) buf, 2);
-      codestream_get ();	/* Flush final byte of enter instruction.  */
+      deprecated_codestream_read ((unsigned char *) buf, 2);
+      deprecated_codestream_get ();	/* Flush final byte of enter instruction.  */
       return extract_unsigned_integer (buf, 2);
     }
   return (-1);
@@ -720,7 +720,7 @@
       addr = fip->frame - 4 - locals;
       for (i = 0; i < 8; i++)
 	{
-	  op = codestream_get ();
+	  op = deprecated_codestream_get ();
 	  if (op < 0x50 || op > 0x57)
 	    break;
 #ifdef I386_REGNO_TO_SYMMETRY
@@ -753,17 +753,17 @@
   if (i386_get_frame_setup (pc) < 0)
     return (pc);
 
-  /* Found valid frame setup -- codestream now points to start of push
+  /* Found valid frame setup -- deprecated_codestream now points to start of push
      instructions for saving registers.  */
 
   /* Skip over register saves.  */
   for (i = 0; i < 8; i++)
     {
-      op = codestream_peek ();
+      op = deprecated_codestream_peek ();
       /* Break if not `pushl' instrunction.  */
       if (op < 0x50 || op > 0x57)
 	break;
-      codestream_get ();
+      deprecated_codestream_get ();
     }
 
   /* The native cc on SVR4 in -K PIC mode inserts the following code
@@ -779,10 +779,10 @@
      function), so we have to skip it to get to the first real
      instruction at the start of the function.  */
 
-  pos = codestream_tell ();
+  pos = deprecated_codestream_tell ();
   for (i = 0; i < 6; i++)
     {
-      op = codestream_get ();
+      op = deprecated_codestream_get ();
       if (pic_pat[i] != op)
 	break;
     }
@@ -791,35 +791,35 @@
       unsigned char buf[4];
       long delta = 6;
 
-      op = codestream_get ();
+      op = deprecated_codestream_get ();
       if (op == 0x89)		/* movl %ebx, x(%ebp) */
 	{
-	  op = codestream_get ();
+	  op = deprecated_codestream_get ();
 	  if (op == 0x5d)	/* One byte offset from %ebp.  */
 	    {
 	      delta += 3;
-	      codestream_read (buf, 1);
+	      deprecated_codestream_read (buf, 1);
 	    }
 	  else if (op == 0x9d)	/* Four byte offset from %ebp.  */
 	    {
 	      delta += 6;
-	      codestream_read (buf, 4);
+	      deprecated_codestream_read (buf, 4);
 	    }
 	  else			/* Unexpected instruction.  */
 	    delta = -1;
-	  op = codestream_get ();
+	  op = deprecated_codestream_get ();
 	}
       /* addl y,%ebx */
-      if (delta > 0 && op == 0x81 && codestream_get () == 0xc3)
+      if (delta > 0 && op == 0x81 && deprecated_codestream_get () == 0xc3)
 	{
 	  pos += delta + 6;
 	}
     }
-  codestream_seek (pos);
+  deprecated_codestream_seek (pos);
 
   i386_follow_jump ();
 
-  return (codestream_tell ());
+  return (deprecated_codestream_tell ());
 }
 
 /* Use the program counter to determine the contents and size of a

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2002-12-01 16:39 [rfa/i386] Make codestream deprecated? Andrew Cagney
@ 2002-12-01 17:02 ` Daniel Jacobowitz
  2002-12-01 17:51   ` Andrew Cagney
  2002-12-07 10:16 ` Mark Kettenis
  2003-01-30  3:31 ` Andrew Cagney
  2 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2002-12-01 17:02 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

On Sun, Dec 01, 2002 at 07:37:43PM -0500, Andrew Cagney wrote:
> Hello,
> 
> The attached is to make it clear that codestream isn't the way to go 
> when trying to improve GDB's performance.  I don't see any urgency in 
> actually removing the code, though.
> 
> The codestream has been supperseeded by a dcache.
> 
> thoughts?

Wouldn't a simple comment serve better than adding a dozen copies of
the word deprecated?


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2002-12-01 17:02 ` Daniel Jacobowitz
@ 2002-12-01 17:51   ` Andrew Cagney
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Cagney @ 2002-12-01 17:51 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> On Sun, Dec 01, 2002 at 07:37:43PM -0500, Andrew Cagney wrote:
> 
>> Hello,
>> 
>> The attached is to make it clear that codestream isn't the way to go 
>> when trying to improve GDB's performance.  I don't see any urgency in 
>> actually removing the code, though.
>> 
>> The codestream has been supperseeded by a dcache.
>> 
>> thoughts?
> 
> 
> Wouldn't a simple comment serve better than adding a dozen copies of
> the word deprecated?

Been there, tried that.  As best I can tell, the only thing that makes 
someone stop and think, is the word deprecated in the name.  Coders 
don't always read the comments, reviewers can't keep track of everything 
that is being eliminated :-/

Andrew



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2002-12-01 16:39 [rfa/i386] Make codestream deprecated? Andrew Cagney
  2002-12-01 17:02 ` Daniel Jacobowitz
@ 2002-12-07 10:16 ` Mark Kettenis
  2003-01-08 20:18   ` Andrew Cagney
  2003-01-30  3:31 ` Andrew Cagney
  2 siblings, 1 reply; 11+ messages in thread
From: Mark Kettenis @ 2002-12-07 10:16 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

Andrew Cagney <ac131313@redhat.com> writes:

> 2002-12-01  Andrew Cagney  <ac131313@redhat.com>
> 
> 	* i386-tdep.c: Replace `codestream' with `deprecated_codestream'.

Sorry, but I'm not really enthousiastic about this patch.  IMHO a
comment explaining the reason why one shouldn't copy this bit of code
would be much better.  I'm willing to rip out this bit of code, and
replace it with something cleaner and simpler, but this "deprication"
is only noise to me.

Mark


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2002-12-07 10:16 ` Mark Kettenis
@ 2003-01-08 20:18   ` Andrew Cagney
  2003-01-08 20:31     ` Daniel Jacobowitz
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Cagney @ 2003-01-08 20:18 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

> Andrew Cagney <ac131313@redhat.com> writes:
> 
> 
>> 2002-12-01  Andrew Cagney  <ac131313@redhat.com>
>> 
>> 	* i386-tdep.c: Replace `codestream' with `deprecated_codestream'.
> 
> 
> Sorry, but I'm not really enthousiastic about this patch.  IMHO a
> comment explaining the reason why one shouldn't copy this bit of code
> would be much better.  I'm willing to rip out this bit of code, and
> replace it with something cleaner and simpler, but this "deprication"
> is only noise to me.

I'll add a comment.  Briefly it will read:

``The deprecated codestream mechanism is entirely redundant.  The dcache 
superseeds it, providing a generic mechanism for caching both 
instruction and data values.  If the dcache has problems or limitations 
than that, and not this code, needs to be fixed.''

While you might think of marking this as deprecated as noice, as I noted 
to Daniel, it has a very real and direct objective:

> Been there, tried that.  As best I can tell, the only thing that makes someone stop and think, is the word deprecated in the name.  Coders don't always read the comments, reviewers can't keep track of everything that is being eliminated :-/

If I don't do this, I find I get a (lets say) less than favourable 
reception when asking a contributor to not [re]use a mechanism 
identified as deprecated via either a comment or bug report.  cf, this 
very code block when cloned into another architecture; or the regcache 
code before I went through and marked much of that as deprecated.

On the other hand, if the code is clearly marked as deprecated, it 
clarifies this situtation - a patch either clearly is or isn't 
using/copying a deprecated mechanism - go directly to goal, do not pass 
go, do not submit patch :-)

So ok?

enjoy,
Andrew



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2003-01-08 20:18   ` Andrew Cagney
@ 2003-01-08 20:31     ` Daniel Jacobowitz
  2003-01-08 21:03       ` Andrew Cagney
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2003-01-08 20:31 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Mark Kettenis, gdb-patches

On Wed, Jan 08, 2003 at 03:18:19PM -0500, Andrew Cagney wrote:
> >Andrew Cagney <ac131313@redhat.com> writes:
> >
> >
> >>2002-12-01  Andrew Cagney  <ac131313@redhat.com>
> >>
> >>	* i386-tdep.c: Replace `codestream' with `deprecated_codestream'.
> >
> >
> >Sorry, but I'm not really enthousiastic about this patch.  IMHO a
> >comment explaining the reason why one shouldn't copy this bit of code
> >would be much better.  I'm willing to rip out this bit of code, and
> >replace it with something cleaner and simpler, but this "deprication"
> >is only noise to me.
> 
> I'll add a comment.  Briefly it will read:
> 
> ``The deprecated codestream mechanism is entirely redundant.  The dcache 
> superseeds it, providing a generic mechanism for caching both 
> instruction and data values.  If the dcache has problems or limitations 
> than that, and not this code, needs to be fixed.''

Except we don't _use_ the dcache, normally.  And my last attempts to
enable it by default met with a pretty crummy reaction.  And the last
time I benchmarked using the dcache I got worse results than without
it.

[And you spelled supersedes incorrectly :)]

> While you might think of marking this as deprecated as noice, as I noted 
> to Daniel, it has a very real and direct objective:
> 
> >Been there, tried that.  As best I can tell, the only thing that makes 
> >someone stop and think, is the word deprecated in the name.  Coders don't 
> >always read the comments, reviewers can't keep track of everything that is 
> >being eliminated :-/
> 
> If I don't do this, I find I get a (lets say) less than favourable 
> reception when asking a contributor to not [re]use a mechanism 
> identified as deprecated via either a comment or bug report.  cf, this 
> very code block when cloned into another architecture; or the regcache 
> code before I went through and marked much of that as deprecated.

Maybe I just have a short memory, but when has that happened?  That
you've pointed out that something was marked deprecated before it was
reused, and gotten a bad reception?

Besides: this is why you should _remove_ them, rather than just
commenting them, if you want them to go away.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2003-01-08 20:31     ` Daniel Jacobowitz
@ 2003-01-08 21:03       ` Andrew Cagney
  2003-01-08 21:13         ` Daniel Jacobowitz
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Cagney @ 2003-01-08 21:03 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Mark Kettenis, gdb-patches


> Besides: this is why you should _remove_ them, rather than just
> commenting them, if you want them to go away.

The implication here being that I'm not removing deprecated code?

The simple reality is that it is not possible for a single individual 
remove all the old code in a single hit.  It takes a group of people 
co-operating, and it takes time.

If you look through the change log you'll notice that deprecated methods 
are being removed from the core of GDB, but the -tdep and -nat files are 
being left largely untouched.  Then, every so often, I or another 
maintainer will expunge the deprecated code from a target being it back 
into synck.  Over time the old code moves to the edges of gdb where it 
wilters and dies.

Andrew



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2003-01-08 21:03       ` Andrew Cagney
@ 2003-01-08 21:13         ` Daniel Jacobowitz
  2003-01-08 21:19           ` Daniel Jacobowitz
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2003-01-08 21:13 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: Mark Kettenis, gdb-patches

On Wed, Jan 08, 2003 at 04:03:33PM -0500, Andrew Cagney wrote:
> 
> >Besides: this is why you should _remove_ them, rather than just
> >commenting them, if you want them to go away.
> 
> The implication here being that I'm not removing deprecated code?

A heck of a lot slower than you're deprecating!  There are now over
1200 references to deprecated constructs in GDB.  That's just a rough
estimate, mind.

I understand why release branches are bad points to compare against for
this, but since I have several handy:
  ~150 deprecated references in gdb 5.3
  ~30 in gdb 5.2

> The simple reality is that it is not possible for a single individual 
> remove all the old code in a single hit.  It takes a group of people 
> co-operating, and it takes time.

Sure, it takes time.  Not all that much time, though; and it doesn't
have to be done all at once.  I prefer to remove it from one target at
a time and then remove support for it entirely.  There should be a
reasonably close alternative available; after all, it's being
deprecated in favor of something.

> If you look through the change log you'll notice that deprecated methods 
> are being removed from the core of GDB, but the -tdep and -nat files are 
> being left largely untouched.  Then, every so often, I or another 
> maintainer will expunge the deprecated code from a target being it back 
> into synck.  Over time the old code moves to the edges of gdb where it 
> wilters and dies.

I don't see this happening.  I see it move to the edge of GDB where it
withers, period.

To come back to the case at hand, this construct has already "moved"
(started out?) to the edge of GDB.  Deprecating it isn't going to help
it wither.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2003-01-08 21:13         ` Daniel Jacobowitz
@ 2003-01-08 21:19           ` Daniel Jacobowitz
  2003-01-08 22:43             ` Andrew Cagney
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Jacobowitz @ 2003-01-08 21:19 UTC (permalink / raw)
  To: Andrew Cagney, Mark Kettenis, gdb-patches

On Wed, Jan 08, 2003 at 04:13:50PM -0500, Daniel Jacobowitz wrote:
> On Wed, Jan 08, 2003 at 04:03:33PM -0500, Andrew Cagney wrote:
> > 
> > >Besides: this is why you should _remove_ them, rather than just
> > >commenting them, if you want them to go away.
> > 
> > The implication here being that I'm not removing deprecated code?
> 
> A heck of a lot slower than you're deprecating!  There are now over
> 1200 references to deprecated constructs in GDB.  That's just a rough
> estimate, mind.
> 
> I understand why release branches are bad points to compare against for
> this, but since I have several handy:
>   ~150 deprecated references in gdb 5.3
>   ~30 in gdb 5.2
> 
> > The simple reality is that it is not possible for a single individual 
> > remove all the old code in a single hit.  It takes a group of people 
> > co-operating, and it takes time.

I feel the need to add:

If you are deprecating things single-handedly, what group of people do
you expect to co-operate?  It's a matter of cleaning up after yourself.

> Sure, it takes time.  Not all that much time, though; and it doesn't
> have to be done all at once.  I prefer to remove it from one target at
> a time and then remove support for it entirely.  There should be a
> reasonably close alternative available; after all, it's being
> deprecated in favor of something.
> 
> > If you look through the change log you'll notice that deprecated methods 
> > are being removed from the core of GDB, but the -tdep and -nat files are 
> > being left largely untouched.  Then, every so often, I or another 
> > maintainer will expunge the deprecated code from a target being it back 
> > into synck.  Over time the old code moves to the edges of gdb where it 
> > wilters and dies.
> 
> I don't see this happening.  I see it move to the edge of GDB where it
> withers, period.
> 
> To come back to the case at hand, this construct has already "moved"
> (started out?) to the edge of GDB.  Deprecating it isn't going to help
> it wither.
> 
> -- 
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2003-01-08 21:19           ` Daniel Jacobowitz
@ 2003-01-08 22:43             ` Andrew Cagney
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Cagney @ 2003-01-08 22:43 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Mark Kettenis, gdb-patches

> On Wed, Jan 08, 2003 at 04:13:50PM -0500, Daniel Jacobowitz wrote:
> 
>> On Wed, Jan 08, 2003 at 04:03:33PM -0500, Andrew Cagney wrote:
> 
>> > 
> 
>> > >Besides: this is why you should _remove_ them, rather than just
>> > >commenting them, if you want them to go away.
> 
>> > 
>> > The implication here being that I'm not removing deprecated code?
> 
>> 
>> A heck of a lot slower than you're deprecating!  There are now over
>> 1200 references to deprecated constructs in GDB.  That's just a rough
>> estimate, mind.
>> 
>> I understand why release branches are bad points to compare against for
>> this, but since I have several handy:
>>   ~150 deprecated references in gdb 5.3
>>   ~30 in gdb 5.2
>> 
> 
>> > The simple reality is that it is not possible for a single individual 
>> > remove all the old code in a single hit.  It takes a group of people 
>> > co-operating, and it takes time.
> 
> 
> I feel the need to add:
> 
> If you are deprecating things single-handedly, what group of people do
> you expect to co-operate?  It's a matter of cleaning up after yourself.

No one is trying to do anything single handedly.  I'm just the luck one 
that gets to follow through some of the dirty work (and put up with the 
resultant flack :-).

Since 5.3 branched, several major changes have gone down, and one is 
still in progress:

- the complaint mechanism was replaced.
Kevin did the bulk of the work here and in the process fixed a 
significant number of bugs!

- the register cache was rewritten
Probably the single biggest nasty bit of work - registers[] - is no 
longer relevant.  The core code no longer uses it.  People are slowly 
expunging -tdep and -nat code of the remainging references.

- the frame's 15 year service
Cleanly integrating things like cfi-frame is made possible; sillyness 
such as re-analyzing a prolog N times can be stopped; the posability of 
having more than one active frame chain (one per thread, hint, hint) is 
real.

The immedate focus is obviously on finishing the new code so that that 
doesn't end up going the same way as many other half finished GDB 
projects (gdb, you'll have to agree, already has too many of these).  To 
that very end, I've finished making `struct frame_info' opaque and I can 
start changing its internals.

However, like others (including you) I also spend time expunging 
deprecated methods.

enjoy,
Andrew



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [rfa/i386] Make codestream deprecated?
  2002-12-01 16:39 [rfa/i386] Make codestream deprecated? Andrew Cagney
  2002-12-01 17:02 ` Daniel Jacobowitz
  2002-12-07 10:16 ` Mark Kettenis
@ 2003-01-30  3:31 ` Andrew Cagney
  2 siblings, 0 replies; 11+ messages in thread
From: Andrew Cagney @ 2003-01-30  3:31 UTC (permalink / raw)
  To: gdb-patches

> Hello,
> 
> The attached is to make it clear that codestream isn't the way to go when trying to improve GDB's performance.  I don't see any urgency in actually removing the code, though.
> 
> The codestream has been supperseeded by a dcache.
> 
> thoughts?
> 
> ok to commit?
> 
> Andrew

Mark, Daniel,

given the current, er, problem with annotation level-two are either of 
you interested in re-considering your view here :-)

Andrew


> 2002-12-01  Andrew Cagney  <ac131313@redhat.com>
> 
> 	* i386-tdep.c: Replace `codestream' with `deprecated_codestream'.



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2003-01-30  3:31 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-12-01 16:39 [rfa/i386] Make codestream deprecated? Andrew Cagney
2002-12-01 17:02 ` Daniel Jacobowitz
2002-12-01 17:51   ` Andrew Cagney
2002-12-07 10:16 ` Mark Kettenis
2003-01-08 20:18   ` Andrew Cagney
2003-01-08 20:31     ` Daniel Jacobowitz
2003-01-08 21:03       ` Andrew Cagney
2003-01-08 21:13         ` Daniel Jacobowitz
2003-01-08 21:19           ` Daniel Jacobowitz
2003-01-08 22:43             ` Andrew Cagney
2003-01-30  3:31 ` Andrew Cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox