Re: dpkg 1.15.6 is slow as hell
Hi!
[ Got the patch few days ago, but didn't test it until today. ]
On Fri, 2010-03-12 at 20:43:29 +0100, Guillem Jover wrote:
> On Fri, 12 Mar 2010 15:57:28 +0000, Colin Watson wrote:
> > I'm worried about the syncing changes though;
> > apparently they're *really* *really* pessimal on some systems, e.g. ext4
> > with data=ordered (which considers rename() as a barrier itself so the
> > fsync() isn't necessary in that configuration). Scott James Remnant
> > reported that it took over an hour to unpack a linux-headers-* package!
> A possible solution could be to do the unpack for all files in a package,
> just leaving the new files as file.dpkg-new, not do either of fsync() or
> replace, and with one pass afterwads fsync() and do the "atomic" (except
> for dirs etc) replace. I guess this might improve a bit the situation for
> packages with lots of files, but not sure how much.
I implemented this and in my tests with linux-headers-2.6.32-4-common
(which contains around 2500 files) on ext3 it reduced the installation
times from 12~ to 5~ seconds (with warm cache).
Could you guys test it on ext4, preferably on such systems which are
taking now over an hour to unpack the package?
Preliminary patch attached.
thanks,
guillem
diff --git a/src/archives.c b/src/archives.c
index e9e3737..da20ffe 100644
--- a/src/archives.c
+++ b/src/archives.c
@@ -388,6 +388,60 @@ linktosameexistingdir(const struct TarInfo *ti, const char *fname,
return true;
}
+void
+tar_deferred_extract(struct pkginfo *pkg)
+{
+ struct fileinlist *cfile;
+ struct filenamenode *usenode;
+ const char *usename;
+
+ for (cfile = pkg->clientdata->files; cfile; cfile = cfile->next) {
+ debug(dbg_eachfile, "deferred extract of '%.255s'", cfile->namenode->name);
+
+ if (!(cfile->namenode->flags & fnnf_deferred_rename))
+ continue;
+
+ debug(dbg_eachfiledetail, "deferred extract needs rename");
+
+ usenode = namenodetouse(cfile->namenode, pkg);
+ usename = usenode->name + 1; /* Skip the leading '/'. */
+
+ setupfnamevbs(usename);
+
+ if (cfile->namenode->flags & fnnf_deferred_fsync) {
+ int fd;
+
+ debug(dbg_eachfiledetail, "deferred extract needs fsync");
+
+ fd = open(fnamenewvb.buf, O_WRONLY);
+ if (fd < 0)
+ ohshite(_("unable to open '%.255s'"), fnamenewvb.buf);
+ if (fsync(fd))
+ ohshite(_("unable to sync file '%.255s'"), fnamenewvb.buf);
+ if (close(fd))
+ ohshite(_("error closing/writing `%.255s'"), fnamenewvb.buf);
+
+ cfile->namenode->flags &= ~fnnf_deferred_fsync;
+ }
+
+ if (rename(fnamenewvb.buf, fnamevb.buf))
+ ohshite(_("unable to install new version of `%.255s'"),
+ cfile->namenode->name);
+
+ cfile->namenode->flags &= ~fnnf_deferred_rename;
+
+ /* CLEANUP: now the new file is in the destination file, and the
+ * old file is in dpkg-tmp to be cleaned up later. We now need
+ * to take a different attitude to cleanup, because we need to
+ * remove the new file. */
+
+ cfile->namenode->flags |= fnnf_placed_on_disk;
+ cfile->namenode->flags |= fnnf_elide_other_lists;
+
+ debug(dbg_eachfiledetail, "deferred extract done and installed");
+ }
+}
+
int tarobject(struct TarInfo *ti) {
static struct varbuf conffderefn, hardlinkfn, symlinkfn;
static int fd;
@@ -661,8 +715,10 @@ int tarobject(struct TarInfo *ti) {
am=(nifd->namenode->statoverride ? nifd->namenode->statoverride->mode : ti->Mode) & ~S_IFMT;
if (fchmod(fd,am))
ohshite(_("error setting permissions of `%.255s'"),ti->Name);
- if (fsync(fd))
- ohshite(_("unable to sync file '%.255s'"), ti->Name);
+
+ /* Postpone the fsync, to try to avoid massive I/O degradation. */
+ nifd->namenode->flags |= fnnf_deferred_fsync;
+
pop_cleanup(ehflag_normaltidy); /* fd= open(fnamenewvb.buf) */
if (close(fd))
ohshite(_("error closing/writing `%.255s'"),ti->Name);
@@ -777,19 +833,25 @@ int tarobject(struct TarInfo *ti) {
* in dpkg-new.
*/
- if (rename(fnamenewvb.buf,fnamevb.buf))
- ohshite(_("unable to install new version of `%.255s'"),ti->Name);
+ if (ti->Type == NormalFile0 || ti->Type == NormalFile0) {
+ nifd->namenode->flags |= fnnf_deferred_rename;
- /* CLEANUP: now the new file is in the destination file, and the
- * old file is in dpkg-tmp to be cleaned up later. We now need
- * to take a different attitude to cleanup, because we need to
- * remove the new file.
- */
+ debug(dbg_eachfiledetail, "tarobject done and installation deferred");
+ } else {
+ if (rename(fnamenewvb.buf, fnamevb.buf))
+ ohshite(_("unable to install new version of `%.255s'"), ti->Name);
+
+ /* CLEANUP: now the new file is in the destination file, and the
+ * old file is in dpkg-tmp to be cleaned up later. We now need
+ * to take a different attitude to cleanup, because we need to
+ * remove the new file. */
- nifd->namenode->flags |= fnnf_placed_on_disk;
- nifd->namenode->flags |= fnnf_elide_other_lists;
+ nifd->namenode->flags |= fnnf_placed_on_disk;
+ nifd->namenode->flags |= fnnf_elide_other_lists;
+
+ debug(dbg_eachfiledetail, "tarobject done and installed");
+ }
- debug(dbg_eachfiledetail,"tarobject done and installed");
return 0;
}
diff --git a/src/archives.h b/src/archives.h
index 7da1561..2390bb8 100644
--- a/src/archives.h
+++ b/src/archives.h
@@ -66,6 +66,7 @@ int unlinkorrmdir(const char *filename);
int tarobject(struct TarInfo *ti);
int tarfileread(void *ud, char *buf, int len);
+void tar_deferred_extract(struct pkginfo *pkg);
bool filesavespackage(struct fileinlist *, struct pkginfo *,
struct pkginfo *pkgbeinginstalled);
diff --git a/src/filesdb.h b/src/filesdb.h
index ac7c274..45a3bf5 100644
--- a/src/filesdb.h
+++ b/src/filesdb.h
@@ -69,6 +69,8 @@ struct filenamenode {
fnnf_elide_other_lists= 000010, /* must remove from other packages' lists */
fnnf_no_atomic_overwrite= 000020, /* >=1 instance is a dir, cannot rename over */
fnnf_placed_on_disk= 000040, /* new file has been placed on the disk */
+ fnnf_deferred_fsync = 000200,
+ fnnf_deferred_rename = 000400,
} flags; /* Set to zero when a new node is created. */
const char *oldhash; /* valid iff this namenode is in the newconffiles list */
struct stat *filestat;
diff --git a/src/processarc.c b/src/processarc.c
index 3a3fb58..722ec9e 100644
--- a/src/processarc.c
+++ b/src/processarc.c
@@ -639,6 +639,8 @@ void process_archive(const char *filename) {
p1[0] = -1;
subproc_wait_check(c1, BACKEND " --fsys-tarfile", PROCPIPE);
+ tar_deferred_extract(pkg);
+
if (oldversionstatus == stat_halfinstalled || oldversionstatus == stat_unpacked) {
/* Packages that were in `installed' and `postinstfailed' have been reduced
* to `unpacked' by now, by the running of the prerm script.
Reply to: