[pacman-dev] [GIT] The official pacman repository branch, master, updated. v3.1.4-163-gfb09d35

Nagy Gabor ngaba at bibl.u-szeged.hu
Thu May 15 05:33:09 EDT 2008

> Nagy Gabor wrote:
> >> commit f43805d875ad5c672afbbfff48bded2087204773
> >> Author: Chantry Xavier<shiningxc at gmail.com>
> >> Date:   Sat May 10 18:47:42 2008 +0200
> >>
> >>      Cleanup usages of alpm_list_find and alpm_list_remove.
> >>
> >>      * remove obsolete and unused *_cmp helper functions like deppkg_cmp
> and
> >>      _alpm_grp_cmp
> >>
> >>      * new alpm_list_remove_str function, used 6 times in handle.c
> >>
> >>      * remove _alpm_prov_cmp / _alpm_db_whatprovides and replace them by
> >>      a more general alpm_find_pkg_satisfiers with a cleaner
> implementation.
> >>      before: alpm_db_whatprovides(db, targ)
> >>      after: alpm_find_pkg_satisfiers(alpm_db_getpkgcache(db), targ)
> >
> > Warning: pkg literal also satisfies pkg. But in most cases we called
> > what_provides if we didn't find a literal.
> >

Yes, there is no problem with this (I just emphasized it). I like the new
function better because I think find_satisfiers is quite a common task. Btw, as
I see, after this patch pacman -S 'kernel26>=2.6.24' automagically works, which
is cool imho.

> I know, it's not exactly equivalent but I think it makes more sense that 
> way and as you noticed, it works the same for our use case.
> >>      * remove satisfycmp and replace alpm_list_find + satisfycmp usage by
> >>      _alpm_find_dep_satisfiers.
> >>      before : alpm_list_find(_alpm_db_get_pkgcache(db), dep, satisfycmp)
> >>      after : _alpm_find_dep_satisfiers(_alpm_db_get_pkgcache(db), dep)
> >
> > Warning: possible slowdown, the old way just stopped after a satisfier
> (which
> > is ideal in checkdeps), now we scan the whole db.
> >
> Right, I knew about that too, I just wanted to keep the code as clean as 
> possible and didn't find another way. Though it might be worth to do 
> some benchmarking / profiling. If it's really too bad, it will have to 
> change.

One more thing, the result of alpm_find_dep_satisfiers should be freed, which is
forgotten (memleak). Remember, that the old list_find was a boolean function, it
was modified in order to handle causingpkg for -Ru. No doubt, this will be
slower. The average slowdown of (successful) dependency check will be about 2x.
(considering only the real calculations, not disk read etc.) The question is
that overall this will be notable or not, this needs testing, indeed. Basically,
I don't prefer clean code over "performance", so I think this should be changed (*).

> >>
> >>      * remove _alpm_pkgname_pkg_cmp, which was used with
> alpm_list_remove,
> >> and
> >>      use _alpm_pkg_find + alpm_list_remove with _alpm_pkg_cmp instead.
> >>
> >
> > Imho this is ugly. First we find it, then we again find it via
> list_remove.
> >
> >
> Yeah, it's not ideal either. But neither are dummy pkg or fake 
> asymmetric cmp functions. I just preferred it like that.
> Imo the real problem here is that our data structures suck and are 
> inefficient. Linear search ftw.

(*) is also holds here. However, performance is not an issue here, just simply
we do an "unneeded" task here, which I find ugly. alpm_list_find compare
functions could be easily killed, but list_remove is quite a complicated
function, thus "reimplementing" won't work. I think in this case keeping
pkgpkgnamecmp was better, even if it was ugly.

An alternative idea: alpm_list_find_node, which returns with an alpm_list_t
node, not the ->data; and reimplement alpm_list_remove_node, but with two param
list (head) and node. This information is enough to cleanly remove that node,
and can be cut from the current list_remove (by splitting that function).


SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu
This mail sent through IMP: http://horde.org/imp/

More information about the pacman-dev mailing list