Re: [PATCH] MI: Free values when updating

[Nick Roberts <nickrob at snap dot net dot nz>]

 > > OK, it's not the right patch but I do think that release_values shouldn't
 > > be called when updating;
 > 
 > Why? If we're about to store a value in varobj->value, we must call
 > release_value on that value, otherwise the value is going to be deleted at a
 > random point in time, and our varobj will be rather useless.

Because there wasn't a call there before your changes.

 > > it's just that this patch stops calling it at other times 
 > > when it's needed.  Without any change, do enable timings (if you have that
 > > patch), create a variable object of a large array and all its children then
 > > repeatedly do "-var-update *".  It should take longer and longer to execute.
 > 
 > Why? Is it because the memory consumption of gdb grows, or because the list
 > of released values grows without ever being cleared, or for some other
 > reason?

The latter, I think.

 >...
 > As I've said, we need to do it to avoid the value to be deleted at random
 > point in time.  So there must be something else? Does you free_values patch
 > makes any difference here?

I think the permanence of values are already guaranteed through value_of_root
and value_of_child.  Maybe trying to release them the second time takes
longer because GDB has to go all the way through the list to find out they're
not there.


 > - Volodya
 > Index: varobj.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/varobj.c,v
 > retrieving revision 1.79
 > diff -u -p -r1.79 varobj.c
 > --- varobj.c	16 Jan 2007 02:12:49 -0000	1.79
 > +++ varobj.c	23 Jan 2007 09:12:07 -0000
 > @@ -1987,11 +1987,7 @@ c_value_of_root (struct varobj **var_han
 >        /* We need to catch errors here, because if evaluate
 >           expression fails we just want to make val->error = 1 and
 >           go on */

This comment is not applicable anymore.

 > -      if (gdb_evaluate_expression (var->root->exp, &new_val))
 > -	{
 > -	  release_value (new_val);
 > -	}
 > -
 > +      gdb_evaluate_expression (var->root->exp, &new_val);
 >        return new_val;
 >      }

I think if you also remove the (3) calls to release_value in c_value_of_child
and cplus_value_of_child this is equivalent to my change (and more tidy).


-- 
Nick                                           http://www.inet.net.nz/~nickrob