[Aptitude-devel] Acquire considered non-threadsafe

[Daniel Burrows]

On Wed, Nov 18, 2009 at 04:03:05PM +0100, Julian Andres Klode <jak at debian.org> was heard to say:
> On Sun, Nov 15, 2009 at 12:42:38PM -0800, Daniel Burrows wrote:
> >   There are some other very nice options involving rewriting or
> > reimplementing the "download queue" logic (i.e., just pkgAcquire), but
> > getting threadsafety does require changing the behavior of
> > pkgAcquire::Item in any event.  I'm assuming that we don't want to have
> > two parallel implementations of this functionality, and that
> > non-backwards-compatible changes are considered unacceptable, in which
> > case the opt-in approach is the only solution I can think of.
> 
> We should really start talking about APT 0.8 sometime soon, and break
> some APIs. The problem I have with Acquire is its low speed when queuing
> items which is probably caused by the use of a linked list (20000 items
> take multiple minutes to be queued, whereas APT2 queues them in less than
> a second[1]).

  Acquire is also really painful to develop against, although that's
more just the general pain of dealing with apt.

  Also, apt2?  When did that happen?

> 	(a) a common abstract base class for the *Summation classes

  Sounds reasonable.

> 	(b) a new cache format which can be resized in reality

  I wonder if apt really needs an mmap'd cache here.  In fact, I wonder
about using sqlite -- I did a little experimentation with it and got
decent performance.  The big problem (from my point of view) would be
teaching client code to treat the database transactionally rather than
as a linked set of in-memory objects, but it way more cleaner and more
flexible than what we have now.

  (I know I came down on this last time it was mentioned here, but I've
done a bit of work with sqlite since then and I have a lot of respect
for it)

  If an mmap'd cache really is desired, I suggest looking at
Boost.Interprocess, which provides all sorts of routines for dealing
with mmap'd memory segments.

> 	(c) new dependency resolver [porting aptitude's one?]
> 	(d) external dependency resolvers
> 	(e) new buildsystem (e.g. raw autotools) and reworked
>             packaging

  Those sound very good.

> 	(f) storing errno inside the error handling objects,
>             for bindings (nice to have errno).

  Please, please, kill the global error queue.  This is a disaster from
a threadsafety point of view, but also just from a general design
perspective.  There's way too much code that does

    if(!_error->PendingError())
      fail();

  when the error could have been caused by something entirely unrelated
to what this code is interested in.

  Put error-handling oun either a return-code basis or an exception
basis, but not this wird intermediate thing where random errors cause
failure.  If you want to know about inner errors, use a logging
framework (I prefer log4cxx).


  I would add one more item:

        (g) use modern C++ coding conventions (at least for new code)

  apt was designed in 1996 or so, and it shows.  Modern C++ is a lot
cleaner, easier to maintain and easier to write than the weird mishmash
of C and "object-oriented programming" that apt is written in.  Also,
a lot of modern C++ programs link against Boost, which provides a ton
of well-designed utility libraries for stuff that you would otherwise
have to write yourself, and that you'd end up writing badly.
(apt-pkg/contrib/ is full of that sort of thing)

> But I don't think anyone wants to do this.

  Personally, I would totally love to clean up the apt code and fix
some of my chronic annoyances with it.  I just don't have the spare
time, and apparently neither does anyone else (except some guy who
thinks it should be written in Perl (!?!?)). :-(

  Daniel