This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 0/4] varobj_list replacement [Re: [patch 4/8] Types GC [varobj_list to all_root_varobjs]]
On Thu, 30 Jul 2009 08:45:51 +0200, Vladimir Prus wrote:
> On Wednesday 29 July 2009 Tom Tromey wrote:
> > I understand from other mail that this patch is a prerequisite to the
> > type GC work. However, I don't understand in what way it is needed. I
> > probably missed something... could you either explain it or tell me
> > where to look?
>
> In fact, I'm lost the big picture as well. If we want to optimize uninstall_variable,
> then the 4/4 patch appears to be the simplest one that does the trick. However, if
> that's a part of some bigger story, I'd be interested to understand it.
The bigger story is:
------------------------------------------------------------------------------
The former Types GC code had to traverse varobj leaves (not just roots as does
current varobj_list()). As I found the current varobj_list() calling
convention is IMO-inconvenient and calling convention of the 'roots' and 'all'
iterators/enumerators of varobjs should be the same I did not want to write
a new function using the IMO-inconvenient calling convention. The new
function was called all_varobjs() in the obsoleted patch:
[patch 8/8] Types GC [varobj]
http://sourceware.org/ml/gdb-patches/2009-05/msg00551.html
So I rather IMO-fixed the calling convention of varobj_list() first so that
the later 'all' varobjs iterator can already use the new calling convention
while keeping the calling convention of the 'roots' and 'all' iterators the
same.
I found the new all_root_varobjs() iterator - patch 3/4 - to be clearly a win
over current varobj_list() - no matter how insignificant change it may be
- that just nobody so far has spent the time doing such code cleanup.
The patches 1/4, 2/4 and 4/4 were later created just to support accepting the
patch 3/4 and I probably would not submit 3/4 if I could imagine any problems
getting it accepted.
Moreover the Types GC changed from the former reference-counting to the
current mark-and-sweep where maybe the leaves traversal is no longer needed.
I have to check it more.
speculation: IIRC it should have been fixing one objfile-invalidating bug
where varobj leaf is using a different objfile due to TYPE_STUB referencing.
But it is a bug out of the critical path for Types GC.
In fact I do not like 1/4 or 2/4 to be accepted, they were created just to get
3/4 accepted. Also I do not think the 4/4 performance improvement is worth
the reviewing time, it was also created just to get 3/4 accepted.
------------------------------------------------------------------------------
As I see 3/4 is not considered as a clear cleanup win. Understood it as
rejected and I will be basing the next patches on the current varobj_list()
calling convention as the preferred one.
Thanks,
Jan