Debian Bug report logs - #74259
dpkg: cache Section strings when parsing

Reply or subscribe to this bug.

Display info messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: branden@deadbeast.net (Branden Robinson)

To: submit@bugs.debian.org

Subject: dpkg: a couple of memory leaks [PATCH]

Date: Sat, 7 Oct 2000 01:20:46 -0500

[Message part 1 (text/plain, inline)]

Package: dpkg
Version: whatever
Severity: normal

----- Forwarded message from Paul Martin <pm-deb@nowster.zetnet.co.uk> -----

From: Paul Martin <pm-deb@nowster.zetnet.co.uk>
To: debian-devel@lists.debian.org
Subject: dpkg reviewed
Date: Sat, 7 Oct 2000 00:35:08 +0100
Delivered-To: branden@localhost.deadbeast.net
Delivered-To: branden@deadbeast.net
Resent-Date: Fri, 6 Oct 2000 18:35:22 -0500 (EST)
X-Envelope-Sender: pm-deb@nowster.zetnet.co.uk
Message-ID: <20001007003508.A18303@nowster.zetnet.co.uk>
User-Agent: Mutt/1.2.5i
Resent-Message-ID: <nuur0.A.CvB.uGm35@murphy>
Resent-From: debian-devel@lists.debian.org
X-Mailing-List: <debian-devel@lists.debian.org> archive/latest/69964
X-Loop: debian-devel@lists.debian.org
Precedence: list
Resent-Sender: debian-devel-request@lists.debian.org

I've been looking over the dpkg lib code wondering why it uses so much
memory. I have Debian on an old laptop with 12MB of RAM.

This is all on i386...

available + status = 5.5MB, but dselect uses considerably more (about
14MB)

Very little digging around got me a couple of memory leaks.
Unfortunately not the whopper I was expecting, but it does reduce the VM
footprint by about a megabyte. Trivial patch for these at the end of
this message.

My next move was to cache the strings copied from Section lines rather
than allocating fresh memory for each new Section line. This freed
another 200k -- there are 79 unique sections but 5765 packages.

Replacing nfstrsave("") in lib/parsehelp.c with a pointer to a static
empty string saved another 130k. Taking "long double d" out of
lib/nfmalloc.c's "union maxalign" saved another 850k. The largest type
stored in nfmalloc space (by libdpkg) is a pointer.

Now, from my studying of the contents of lib/, the pool of nfmalloc
memory should be approximately equal to the combined sizes of available
and status (ie. 5.3MB). However the nfmalloc pool is nearly 6.8MB in
size after my little tweaks. The difference amounts to about 275 bytes
per package.

Anybody want to speculate where the difference goes?

(Passing comment: dselect is evil. It makes another copy of most of the
data that's already in the data segment.)


--- lib/fields.c~	Mon Oct 25 19:52:12 1999
+++ lib/fields.c	Fri Oct  6 23:38:34 2000
@@ -377,5 +377,6 @@
     if (!*p) break;
     p++; while (isspace(*p)) p++;
   }
+  varbuffree(&depname); varbuffree(&version);
 }
 
--- lib/dump.c~	Thu Oct 21 11:38:39 1999
+++ lib/dump.c	Fri Oct  6 23:40:08 2000
@@ -222,6 +222,7 @@
   varbufaddc(&vb,'\0');
   if (fputs(vb.buf,file) < 0)
     ohshite("failed to write details of `%.50s' to `%.250s'", pigp->name, filename);
+  varbuffree(&vb);
 }
 
 void writedb(const char *filename, int available, int mustsync) {
 

-- 
Paul Martin <pm@zetnet.net> (work)
  <pm@nowster.zetnet.co.uk> (home)


-- 
To UNSUBSCRIBE, email to debian-devel-request@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org


----- End forwarded message -----

-- 
G. Branden Robinson             |    Q: How does a Unix guru have sex?
Debian GNU/Linux                |    A: unzip;strip;touch;finger;mount;fsck;
branden@debian.org              |       more;yes;fsck;fsck;fsck;umount;sleep
http://www.debian.org/~branden/ |

[Message part 2 (application/pgp-signature, inline)]

Message #12 received at 74259@bugs.debian.org (full text, mbox, reply):

From: Adam Heath <adam@doogie.org>

To: Branden Robinson <branden@deadbeast.net>, Paul Martin <pm-deb@nowster.zetnet.co.uk>

Cc: <74259@bugs.debian.org>

Subject: Re: dpkg: a couple of memory leaks [PATCH]

Date: Sat, 24 Aug 2002 17:24:55 -0500 (CDT)

Sorry for taking so long to respond.  Some of the suggestions in this bug
report have been put into dpkg long ago, but no response was ever given to
this bug.  Doing so now.

Please keep this bug open, until all ideas mentioned below are applied.

On Sat, 7 Oct 2000, Branden Robinson wrote:

> My next move was to cache the strings copied from Section lines rather
> than allocating fresh memory for each new Section line. This freed
> another 200k -- there are 79 unique sections but 5765 packages.

This has not yet been done in the current dpkg code base.  Do you(Paul) still
have the code you used to do this?

> Replacing nfstrsave("") in lib/parsehelp.c with a pointer to a static
> empty string saved another 130k. Taking "long double d" out of
> lib/nfmalloc.c's "union maxalign" saved another 850k. The largest type
> stored in nfmalloc space (by libdpkg) is a pointer.

nfstrsave("") is now put into a static var, and that is used over and over.

nfmalloc is now based in glibc obstacks, so the union thing no longer applies.

> Now, from my studying of the contents of lib/, the pool of nfmalloc
> memory should be approximately equal to the combined sizes of available
> and status (ie. 5.3MB). However the nfmalloc pool is nearly 6.8MB in
> size after my little tweaks. The difference amounts to about 275 bytes
> per package.
>
> Anybody want to speculate where the difference goes?

Nat at this time, no.

>
> (Passing comment: dselect is evil. It makes another copy of most of the
> data that's already in the data segment.)
>
>
> --- lib/fields.c~	Mon Oct 25 19:52:12 1999
> +++ lib/fields.c	Fri Oct  6 23:38:34 2000
> @@ -377,5 +377,6 @@
>      if (!*p) break;
>      p++; while (isspace(*p)) p++;
>    }
> +  varbuffree(&depname); varbuffree(&version);
>  }
>

This patch no longer is valid.  The function being manipulated above uses 2
static vars(depname and version), and saves their memory between calls.

> --- lib/dump.c~	Thu Oct 21 11:38:39 1999
> +++ lib/dump.c	Fri Oct  6 23:40:08 2000
> @@ -222,6 +222,7 @@
>    varbufaddc(&vb,'\0');
>    if (fputs(vb.buf,file) < 0)
>      ohshite("failed to write details of `%.50s' to `%.250s'", pigp->name, filename);
> +  varbuffree(&vb);
>  }
>
>  void writedb(const char *filename, int available, int mustsync) {

This has been part of dpkg for awhile now.

Message #25 received at 74259@bugs.debian.org (full text, mbox, reply):

From: Branden Robinson <branden@debian.org>

To: Esteban Manchado Velázquez <zoso@demiurgo.org>, 74259@bugs.debian.org

Subject: Re: About Bug#74259

Date: Mon, 3 Nov 2003 14:02:40 -0500

[Message part 1 (text/plain, inline)]

On Sun, Nov 02, 2003 at 10:45:51PM +0000, Esteban Manchado Velázquez wrote:
>    Just out of curiosity: there is an old bug in dpkg (#74259 in the BTS),
> originally opened by you. After some comments, it seems the patch sent was no
> longer valid, but also no longer necessary AFAIUI. It was closed and reopened
> by you, without any explanation (in a mass-reopen operation). I'm just not
> sure you didn't reopened it by error, because it seems the problem is solved
> (both memory leaks gone). Can you tell me if it was reopened by error, and,
> if not, please comment something in the BTS so nobody has the same doubt
> again? :-)
> 
>    Perhaps it's because the Section lines caching? In that case, has anyone
> contacted (again) Paul Martin, so we can sort it out?

I believe the explanation lies in Adam Heath's last mail to the bug:

"Please keep this bug open, until all ideas mentioned below are
applied."

You'll have to talk to him about that.

-- 
G. Branden Robinson                |     If you have the slightest bit of
Debian GNU/Linux                   |     intellectual integrity you cannot
branden@debian.org                 |     support the government.
http://people.debian.org/~branden/ |     -- anonymous

[signature.asc (application/pgp-signature, inline)]

Send a report that this bug log contains spam.