> Hi,
>
> So Marius came up with the "but having to sometimes use -Syy on -current
> is annoying", and in the past we were generally blamed for having a slow
> -Sy / -Su operation.
>
> One possible solution is to avoid unpacking the fdb at all, and use
> libarchive to read from the fdb when needed. This obviously makes random
> access within the fdb hard, but as long as reading the whole fdb at once
> is about as fast as reading a single entry from the old db, I would not
> worry.
>
> So here is what I have:
>
> 1) -Si is a bit slower now:
>
> $ time pacman -Si glibc
> real    0m2.957s
>
> $ time ../src/pacman-g2/pacman-g2 --config root/etc/pacman-g2.conf -r root
> -Si glibc
> real    0m3.262s
>
> 2) -Sy / -Su is tad faster:
>
> I did not benchmark -Sy, but it basically depends on your bandwidth,
> since it's just about downloading frugalware-current.fdb to
> /var/lib/pacman-g2.
>
> A noop -Su is way faster:
>
> $ time sudo pacman -Su
> :: Starting local database upgrade...
> real    1m20.296s
>
> $ time sudo ../src/pacman-g2/pacman-g2 --config root/etc/pacman-g2.conf -r
> root -Su
> :: Starting local database upgrade...
> real    0m3.866s
>
> And given that we always read from the fdb, data is always up to date,
> no need to use -Syy in case a pkg was uploaded with the same pkgrel
> twice.
>
> Needless to say, the testsuite passes.
>
> Why I did not push these patches directly to git:
>
> 1) A proof-read of the third patch from others (Michel? :) ) would be
> nice. The first two is quite trivial at the moment.

Well most of this code is too much cryptic in some place for me.
But from code point I see 2-3 things that annoys me.
- on first patch you use strcmp to check if the database is local, and in
second patch you made the same and even made a fuction out of it. Maybe it
should be defined for real in db.c as a function.
- _pacman_archive_fgets I see this as a bad practice. From api point of
view it seems that libarchive don't provide an archive_entry to file
descriptor open. This would allow the reusage of all the standard libc.
Maybe you should ask libarchive maintainers to provide such function.
- Lats patch seems really cryptic, and I see quite some duplications
couldn't it be factorised? (Also those fgets to _pacman_archive_fgets
would disapears somehow if 2 is apply)

> 2) I ran out of time, updating the rest of _pacman_db_* functions is not
> strictly necessary, but it's ugly to call closedir() on a libarchive
> handle.
>
> 3) I think further optimisation of _pacman_archive_fgets is possible, if
> we do not each character separately (so -Si would not be a slowdown,
> either).
>
> 4) Other issues: islocal() uses strcmp() at the moment, while it could
> just compare to the handle->local_db pointer, seeking to the start of
> the fdb is currently done by closing and reopening it, which is ugly.

Other test worth to try, have you tried some testing on using an
uncomressed version of the .fdb ? (a plain uncompressed archive) It just
to have quite some extra numbers to compare with.

_______________________________________________
Frugalware-devel mailing list
[email protected]
http://frugalware.org/mailman/listinfo/frugalware-devel

Reply via email to