tiemann@SUN.COM (Michael Tiemann) (12/01/89)
These diffs fix a performance bug I wrote into GDB about two years
ago. The bug is that when you ask GDB to print the value of `foo', it
has to look up `foo' in the various contexts that `foo' could belong
to. In the case of member functions, it must check whether `foo' is a
field of `this'. In the original code I wrote, I saved the wrong
cycles when I indirected `this' to get the structure definition so I
could look up the name in the type. In parallel with doing the
indirection for the name lookup, I had GDB go and fetch the object
from memory, so its value would be close at hand on a hit. Well, this
leads to slightly reduced performance in the case where the object is
small, it's just a call to ptrace that didn't need to be made.
However, I was wondering why GDB was taking so long to find that a
particular name was not in scope, and I found that my object, all 200K
of it, was being ptraced into my address space just to do the search!
Yow!
teacake% diff -c2 valops.c~ valops.c
*** valops.c~ Sun Nov 5 10:04:33 1989
--- valops.c Thu Nov 30 21:36:54 1989
***************
*** 1009,1017 ****
while (TYPE_CODE (t) == TYPE_CODE_PTR || TYPE_CODE (t) == TYPE_CODE_REF)
! {
! arg1 = value_ind (arg1);
! COERCE_ARRAY (arg1);
! t = VALUE_TYPE (arg1);
! }
if (TYPE_CODE (t) == TYPE_CODE_MEMBER)
--- 1009,1013 ----
while (TYPE_CODE (t) == TYPE_CODE_PTR || TYPE_CODE (t) == TYPE_CODE_REF)
! t = TYPE_TARGET_TYPE (t);
if (TYPE_CODE (t) == TYPE_CODE_MEMBER)
***************
*** 1030,1036 ****
char *t_field_name = TYPE_FIELD_NAME (t, i);
if (t_field_name && !strcmp (t_field_name, name))
! {
! return 1;
! }
}
if (TYPE_N_BASECLASSES (t) == 0)
--- 1026,1030 ----
char *t_field_name = TYPE_FIELD_NAME (t, i);
if (t_field_name && !strcmp (t_field_name, name))
! goto success;
}
if (TYPE_N_BASECLASSES (t) == 0)
***************
*** 1038,1042 ****
t = TYPE_BASECLASS (t, 1);
- VALUE_TYPE (arg1) = t; /* side effect! */
}
--- 1032,1035 ----
***************
*** 1048,1052 ****
/* Destructors are a special case. */
if (destructor_name_p (name, t))
! return 1;
while (t)
--- 1041,1045 ----
/* Destructors are a special case. */
if (destructor_name_p (name, t))
! goto success;
while (t)
***************
*** 1064,1067 ****
--- 1057,1069 ----
}
return 0;
+
+ success:
+ t = VALUE_TYPE (arg1);
+ while (TYPE_CODE (t) == TYPE_CODE_PTR || TYPE_CODE (t) == TYPE_CODE_REF)
+ {
+ arg1 = value_ind (arg1);
+ COERCE_ARRAY (arg1);
+ t = VALUE_TYPE (arg1);
+ }
}
teacake%
This could still be done better: `check_field' should not read in the
whole object. It should be up to the caller to decide whether to
ptrace in the memory. Right now the following code from
`lookup_symbol' (symtab.c)
/* C++: If requested to do so by the caller,
check to see if NAME is a field of `this'. */
if (is_a_field_of_this)
{
int v = (int) value_of_this (0);
*is_a_field_of_this = 0;
if (v && check_field (v, name))
{
*is_a_field_of_this = 1;
return 0;
}
}
is the only place where check_field is called, and it clearly is not
using its value. Since the published interface of `check_field' does
not indicate that it brings values in from disk, I advocate removing
that paging-in mis-feature altogether, but I'll leave the final
decision to Jim.
Michael