[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH] libdpkg: use mmap if available to parse package info files


On Tue, 2010-05-25 at 13:36:16 -0500, Jonathan Nieder wrote:
> Use mmap instead of slurping entire "Packages" or "available" files
> into memory.  This should reduce memory pressure on low-memory
> machines.
> Reported-by: Bill Allombert <Bill.Allombert@math.u-bordeaux1.fr>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Bill Allombert wrote:
> > Maybe this is because I reran autoreconf
> > before compiling or because I do not use the same version of autoconf/automake
> > as you. 
> Maybe some versions of autoconf include an implied AC_CHECK_FUNCS(mmap)
> somehow.

I just checked and it used to be implied by gettext (through libintl),
but that changed when it got switched to use the system library instead
of the embedded one. And I didn't catch the missing check for HAVE_MMAP
in 2ab983d2c0ef13ed242480d0f92a98f42b749a16.

> > In any case, we certainly have mmap support, so it seems like a bug
> > if it was not detected on your system.
> Yes, agreed.  Thanks for noticing.
> It would be nice to get some benchmarks: how does this affect dpkg’s
> speed and memory usage?

I actually had this on my TODO, from when I spotted the problem while
doing the other mmap related changes. The thing is, the current mmap
support is unreliable as it does not handle SIGBUS which can happen due
to I/O errors for example, also some file systems do not support mmap.
At the time, I did some tests and there was no performance difference,
given that both implementations need to read the whole file eventually.
I did them on a machine with enough memory to hold the files though, it
would still be interesting to check on memory constrained systems,
which is where any difference will be noticeable. But even then I think
I'd just prefer switching the code to use chunked I/O instead, which
should be as reliable, and use consistently way less memory at any
given time.

Committed now a temporary fix e844672595246c81f4a8e45ca842dc6aa3a4dbc8,
which might in the future derive into removing the mmap code.


Reply to: