[patch] remove extraneous check_typedef () call in c_val_print ()

[patch] remove extraneous check_typedef () call in c_val_print ()

[Jason Molenda]

Hello, my name is Jason Molenda, here is my first patch submission to 
GDB.  I've heard that my employer has a copyright assignment on file 
already, so I expect those details are all in order.  If I've missed 
some bit of documentation about how to submit a patch properly, please 
let me know.

This patch fixes the case where c_val_print() calls check_typedef() 
twice for some common cases.  On MacOS X, check_typedef() can be very 
expensive for some of our symbols, so calling this function any more 
than necessary is to be avoided.
The code in c_val_print() looks like this:

   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_ARRAY:
       elttype = check_typedef (TYPE_TARGET_TYPE (type));
[...]
       /* Array of unspecified length: treat like pointer to first 
elt.  */
       addr = address;
       goto print_unpacked_pointer;

     case TYPE_CODE_PTR:
[...]
       elttype = check_typedef (TYPE_TARGET_TYPE (type));
       if (TYPE_CODE (elttype) == TYPE_CODE_METHOD)
         {
[...]
        else
         {
           addr = unpack_pointer (type, valaddr + embedded_offset);
         print_unpacked_pointer:
           elttype = check_typedef (TYPE_TARGET_TYPE (type));

           if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)


In short, we have two code paths that are followed -- if it's a 
TYPE_CODE_ARRAY,
we call check_typedef, and in some cases we jump (goto) to the middle of 
the TYPE_CODE_PTR case for printing.  For TYPE_CODE_PTR, we call 
check_typedef, then we have the goto label, then we call check_typedef 
again.  This second check_typedef was necessary long ago when 
check_typedef was only run for _some_ cases in the TYPE_CODE_ARRAY 
case.  The check_typedef call was moved out of a conditional statement 
as a part of the HP merge (ChangeLog appended below) in 1998, making 
this second check_typedef call in TYPE_CODE_PTR redundant.

On OS X, we have many opaque struct pointers.  Programs get these 
pointers to e.g. window handles and pass the pointers to various toolkit 
functions.  To discourage developers from touching the internals, there 
are no symbols provided for these opaque structures.  These opaque 
struct pointers are common data types in OS X programs.

The end result is that you have local variables with this type, and if 
you have a GUI showing the locals after every 'step', gdb is forced to 
grub through all the symbols looking for this opaque struct at every 
step.  This is very very slow.  And doing it _twice_ was even 
slower. :-)  We'll probably have to look at cacheing the fact that the 
struct pointer has no definition some time in the future (it's still far 
too slow), but this is a reasonable first step -- the lowest of the low 
hanging fruit, you might say.

Jason

Thu Dec 31 16:54:30 1998  David Taylor  <taylor@texas.cygnus.com>

[...]

         * c-valprint.c (c_val_print): Add new parameter embedded_offset.
         Add embedded_offset to valaddr in function calls.  fix calls to
         val_print, and cp_print_value_fields.  Attempt to determine the
         real type of the object to be printed.  fixed call to
         cp_print_value_fields.  process TYPE_CODE_METHOD as well.  moved
         call to check_typedef out of conditional.  add embedded offset
         param to val_print call.

Re: [patch] remove extraneous check_typedef () call in c_val_print ()

[Jason Molenda]

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

On Friday, August 31, 2001, at 02:39 PM, Jason Molenda wrote:

> Hello, my name is Jason Molenda, here is my first patch submission to 
> GDB.  I've heard that my employer has a copyright assignment on file 
> already, so I expect those details are all in order.  If I've missed 
> some bit of documentation about how to submit a patch properly, please 
> let me know.
>

Including the patch would be a good first step, I suppose.  BTW I did 
run the gdb testsuite with this patch and there are no additional 
regressions.  Here's the patch:


[-- Attachment #2: pa --]
[-- Type: text/x-diff, Size: 665 bytes --]

2001-08-31  Jason Molenda  (jmolenda@apple.com)

	* c-valprint.c (c_val_print):  Second call to check_typedef ()
	is no longer necessary.

Index: c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.9
diff -u -p -r1.9 c-valprint.c
--- c-valprint.c	2001/04/27 00:19:09	1.9
+++ c-valprint.c	2001/08/31 21:29:04
@@ -168,7 +168,6 @@ c_val_print (struct type *type, char *va
 	{
 	  addr = unpack_pointer (type, valaddr + embedded_offset);
 	print_unpacked_pointer:
-	  elttype = check_typedef (TYPE_TARGET_TYPE (type));
 
 	  if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
 	    {

Re: [patch] remove extraneous check_typedef () call in c_val_print ()

[Michael Snyder]

Jason Molenda wrote:
> 
> On Friday, August 31, 2001, at 02:39 PM, Jason Molenda wrote:
> 
> > Hello, my name is Jason Molenda, here is my first patch submission to
> > GDB.  I've heard that my employer has a copyright assignment on file
> > already, so I expect those details are all in order.  If I've missed
> > some bit of documentation about how to submit a patch properly, please
> > let me know.
> >
> 
> Including the patch would be a good first step, I suppose.  BTW I did
> run the gdb testsuite with this patch and there are no additional
> regressions.  Here's the patch:

Approved and committed.

Re: [patch] remove extraneous check_typedef () call in c_val_print ()

[Andrew Cagney]

jason,

i've sent you info on getting an account, once that is done, feel free 
to add your self to the write - after - approval list.

andrew

2001-08-31  Jason Molenda  (jmolenda@apple.com)

         * c-valprint.c (c_val_print):  Second call to check_typedef ()
         is no longer necessary.

Index: c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.9
diff -u -p -r1.9 c-valprint.c
--- c-valprint.c        2001/04/27 00:19:09     1.9
+++ c-valprint.c        2001/08/31 21:29:04
@@ -168,7 +168,6 @@ c_val_print (struct type *type, char *va
         {
           addr = unpack_pointer (type, valaddr + embedded_offset);
         print_unpacked_pointer:
-         elttype = check_typedef (TYPE_TARGET_TYPE (type));

           if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
             {