[Aptitude-devel] aptitude-qt VCS usage and code review

[Daniel Burrows]

On Fri, Jul 9, 2010 at 2:14 AM, Piotr Galiszewski <piotr at galiszewski.pl>wrote:

> 2010/7/9 Daniel Burrows <dburrows at google.com>:
> >   These are the ways of retrieving package information from a package
>
> that spring to mind immediately:
> >   1) Fully eager: as soon as the Package is created, copy all the values
> > from
> >       the PkgIterator over into the Package object (I believe the GTK+
> code
> >       effectively does this).
> >   2) Fully lazy: the first time a member of Package is accessed, copy all
> > the
> >       values from the PkgIterator into it (what the current Qt code
> does).
> >   3) Memberwise lazy without caching: just store a PkgIterator; when a
> >       component of the Package is requested, return the corresponding
> >       member of the PkgIterator (what the curses frontend does).
> >   4) Memberwise lazy with caching: when a component of the Package is
> >       requested, copy the corresponding member of the PkgIterator into
> the
> >       Package object and return it (I don't think any aptitude code does
> > this).
> >   Which of these did you try?  Do you have numbers showing how big an
> > improvement you saw?  Could you try some of the others to compare?
>
> I tried second and third points. Do not have any hard numbers. My
> assumptions were only based on feeling and time of response from GUI.
> I do not know how to provide numbers, but I will try.


  I might just instrument the GUI to run whatever you're testing immediately
on startup, then exit, and use the "time" command to check how long it took.
An alternative, maybe better, would be to save the system time before and
after the process you're interested in and log it (say, using LOG_()).

  When I'm timing stuff like this I generally run it twice to eliminate
cache
effects (the time on an empty cache is somewhat interesting, but I think
that normally a user of the GUI interface will have the package database
cached).


> The speed was
> the easiest to observe during sorting and searching packages. On the
> other hand, I think that returning of the most members will be short.
> Only description loading and maybe something more is time consuming. I
> will experiment with this
>

  My main concern is that I don't want to get a bunch of complicated
code to optimize something that doesn't need to be optimized.  But then,
maybe it'll turn out that it does need to be optimized after all...


> >   Is there any way to lose the QFoo types in Package?  I'm not sure we
> > want to reuse it directly, but there might be a way to solve this problem
> > more generally, as long as it doesn't require the use of Qt-specific (or
> > GTK-specific) types.
>
> Yes. There are only QStrings and QStringLists. std::string and
> std::list could also be used. Moreover, I think that this class
> requires much more thinking. Maybe it will be better do extend
> PackageInformation class (and rename this to package_information or
> package ;) ) ?


  Yeah, that could be a better place to start.


> There is also one more problem I am not entirely sure
> hot is should be solved. What should be done after cache updating?
> Packages could be changed so the state of package class also should be
> updated. Maybe package objects should be stored in map/hashtable in
> packages_pool instead of simple list?


  You need to deal both with changes to package state, and with the
whole cache being discarded and reloaded.

  For changes to package state, you need some sort of map from
package iterators to objects in your GUI model.  The backend supplies
you with a set of packages whose state has changed, and you just
need to find them and update them.  In the GTK+ code, this is a map
from packages to iterators in the GTK+ tree model (we use a stock
one).  If you're responsible for storing your own list, I'd recommend
using boost::multi_index_container for this case (one index for the
display order of the elements, another for finding elements by package
iterator).

  In the GTK+ and curses frontends, we deal with the cache being
reloaded by throwing away all the currently displayed packages
and rebuilding the view from scratch.  The only real alternative would
be to somehow merge the new cache into the display, maybe on the
basis of name and/or version strings?  I don't think it's worth the
trouble for your first version.

  Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/aptitude-devel/attachments/20100709/8b9187dd/attachment.htm>