XPost: linux.debian.bugs.dist
From: [email protected]

Package: dpkg
Version: 1.20.12
Severity: wishlist

Dear Maintainer,

on many of my systems using nvme drives or even sata ssds, the
FS_IOC_FIEMAP scan that dpkg does is not only superfluous, but with a
large number of packages installed, absolutely dominates the installation
time (based on profiling) - essentially, dpkg does nothing else during the preparing/unpacxking/selecting steps.

and while being a very useful optimisation for spinning rust drives, when installing many small packages, the scan is apparently done for every
package unpack, when, again, on modern systems, everything is in the cache aftzer the first pass.

could you consider making this scan optional (i.e. off-switchable via an option)? this way, users can disable it in dpkg.cfg on those systems where
it matters, while still retaining it by default (I do not think trying to autodetect ssds is a good idea here, and a switch should be trivial to implement).

thanks a lot for your consideration!

PS: it might also be a good idea to reconsider the whole scan - fiemap
often creates just as many seeks as a stat or an open, especially on more complicated filesystems. In fact, sorting by inode numbers (which are essentially free info) is often a good approximation for locality on many filesystems and might be a good replacement strategy.

-- Package-specific info:
System tainted due to merged-usr-via-aliased-dirs.

-- System Information:
Debian Release: 11.7
APT prefers stable-updates
APT policy: (990, 'stable-updates'), (990, 'stable-security'), (990, 'stable'), (500, 'unstable-debug'), (500, 'testing-debug'), (500, 'stable-debug'), (500, 'unstable'), (500, 'testing'), (1, 'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386, x32

Kernel: Linux 6.1.27-schmorp (SMP w/24 CPU threads; PREEMPT)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_USER, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_IE.UTF-8, LC_CTYPE=en_IE.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages dpkg depends on:
ii libbz2-1.0 1.0.8-4
ii libc6 2.31-13+deb11u6
ii liblzma5 5.2.5-2.1~deb11u1
ii libselinux1 3.1-3
ii tar 1.34+dfsg-1
ii zlib1g 1:1.2.11.dfsg-2+deb11u2

dpkg recommends no packages.

Versions of packages dpkg suggests:
ii apt 2.2.4
pn debsig-verify <none>

-- Configuration Files:
/etc/dpkg/dpkg.cfg changed [not included]

-- no debconf information

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)

XPost: linux.debian.bugs.dist
From: [email protected]

Hi!

On Thu, 2023-05-04 at 06:16:52 +0200, Marc Lehmann wrote:

Package: dpkg
Version: 1.20.12
Severity: wishlist

on many of my systems using nvme drives or even sata ssds, the
FS_IOC_FIEMAP scan that dpkg does is not only superfluous, but with a
large number of packages installed, absolutely dominates the installation time (based on profiling) - essentially, dpkg does nothing else during the preparing/unpacxking/selecting steps.

and while being a very useful optimisation for spinning rust drives, when installing many small packages, the scan is apparently done for every
package unpack, when, again, on modern systems, everything is in the cache aftzer the first pass.

Hmm, right, whenever we update and write the files list file on disk
we invalidate the in-core lists and force a reload for that package,
so the scan might indeed be performed for at least each unpacked
package when processing each subsequent one.

The other potential cause for the entire database being loaded for
each package you might also be seeing is because apt tends to split
essential package installation into individual runs to reduce
potential breakage (AFAIR).

could you consider making this scan optional (i.e. off-switchable via an option)? this way, users can disable it in dpkg.cfg on those systems where
it matters, while still retaining it by default (I do not think trying to autodetect ssds is a good idea here, and a switch should be trivial to implement).

Yes, AFAIR at the time when implementing this, autodetection was
considered, but was quickly discarded as unfeasible/unportable and
error-prone, and we opted for performing this unconditionally.

I've now locally implemented this via a new force option enabled by
default, that I'm tentatively calling --force-mechanical-load, but I'll
ponder about the name because it's the first thing that came to mind,
and I'm not fully convinced. If you happen to have proposals I'm all
ears. :)

But looking further, the scan is not ignoring packages that already
have a valid loaded files list file (although it should already be
caching the physical offset if it was found and then skip the scan),
so I'll skip those, which might also help by substantially reducing
the work being done on both HDD and NVME/SSD.

I might also look into avoiding the scan completely if there are only
few files list files to reload (instead of say the entire lot), but
I'm not sure how much that will help once/if it is only done for the
few packages that need a reload (if the current caching is not effective
for some reason, then I'd assume it might not help a lot?). And thinking
about it perhaps even the force option is not really necessary after
the scan fix anyway? (But it would still help in the apt splitting case mentioned above though.)

I'm attaching the change to skip unnecessary optimization scan, in
case you want to give it a go, although I've only compile tested it
for now.

PS: it might also be a good idea to reconsider the whole scan - fiemap
often creates just as many seeks as a stat or an open, especially on more complicated filesystems. In fact, sorting by inode numbers (which are essentially free info) is often a good approximation for locality on many filesystems and might be a good replacement strategy.

Perhaps, I would need to check the context from when it was added as
I cannot recall whether this option was considered and discarded,
and otherwise it would need to be benchmarked on a bunch of different filesystems and types of disks to see whether there would be
significant performance degradation/regression.

Thanks,
Guillem

From b93783b16b75eea334d82221cf5a71af5ba1ff99 Mon Sep 17 00:00:00 2001
From: Guillem Jover <[email protected]>
Date: Mon, 22 May 2023 01:26:15 +0200
Subject: [PATCH] libdpkg: Do not unnecessarily optimize the db-fsys load for
loaded files

If the files list file is valid and will not be reloaded there is no
point in including it in the optimization load logic. Skip them as
we do when trying to (re)load these files. For the posix_fadvise() case
this is a clear fix, for the fiemap case we are supposedly already
caching the physical offset, so in case we already found it these
packages should have been skipped, but if we failed to fetch the
fiemap information then that might cause the wasted operations for
all packages.

FIXME: check why the caching might not be currently effecting.

Closes: #1035486
---
lib/dpkg/db-fsys-files.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/lib/dpkg/db-fsys-files.c b/lib/dpkg/db-fsys-files.c
index fc67d1d20..c16b69487 100644
--- a/lib/dpkg/db-fsys-files.c
+++ b/lib/dpkg/db-fsys-files.c
@@ -194,6 +194,9 @@ pkg_files_optimize_load(struct pkg_array *array)
const char *listfile;
int fd;

+ if (pkg->files_list_valid)
+ continue;
+
if (pkg->status == PKG_STAT_NOTINSTALLED ||
pkg->files_list_phys_offs != 0)
continue;
@@ -233,6 +236,9 @@ pkg_files_optimize_load(struct pkg_array *array)
const char *listfile;
int fd;

+ if (pkg->files_list_valid)
+ continue;
+
listfile = pkg_infodb_get_file(pkg, &pkg->installed, LISTFILE);

fd = open(listfile, O_RDONLY | O_NONBLOCK);
--
2.40.1

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)

XPost: linux.debian.bugs.dist
From: [email protected]

Hi!

[ Leaving most mail context for Colin (in CC). ]

On Mon, 2023-05-22 at 01:47:27 +0200, Guillem Jover wrote:

On Thu, 2023-05-04 at 06:16:52 +0200, Marc Lehmann wrote:

Package: dpkg
Version: 1.20.12
Severity: wishlist

on many of my systems using nvme drives or even sata ssds, the FS_IOC_FIEMAP scan that dpkg does is not only superfluous, but with a
large number of packages installed, absolutely dominates the installation time (based on profiling) - essentially, dpkg does nothing else during the preparing/unpacxking/selecting steps.

and while being a very useful optimisation for spinning rust drives, when installing many small packages, the scan is apparently done for every package unpack, when, again, on modern systems, everything is in the cache aftzer the first pass.

Hmm, right, whenever we update and write the files list file on disk
we invalidate the in-core lists and force a reload for that package,
so the scan might indeed be performed for at least each unpacked
package when processing each subsequent one.

The other potential cause for the entire database being loaded for
each package you might also be seeing is because apt tends to split
essential package installation into individual runs to reduce
potential breakage (AFAIR).

I guess the other one is whether the filesystem in use does not
support fiemap, in which case the ioctl will fail, and the current
caching logic would retrigger the ioctls for each iteration. The
patch I attached previously should fix this case, so I'm going to
include it no matter what on my next push.

could you consider making this scan optional (i.e. off-switchable via an option)? this way, users can disable it in dpkg.cfg on those systems where it matters, while still retaining it by default (I do not think trying to autodetect ssds is a good idea here, and a switch should be trivial to implement).

Yes, AFAIR at the time when implementing this, autodetection was
considered, but was quickly discarded as unfeasible/unportable and error-prone, and we opted for performing this unconditionally.

I've now locally implemented this via a new force option enabled by
default, that I'm tentatively calling --force-mechanical-load, but I'll ponder about the name because it's the first thing that came to mind,
and I'm not fully convinced. If you happen to have proposals I'm all
ears. :)

But looking further, the scan is not ignoring packages that already
have a valid loaded files list file (although it should already be
caching the physical offset if it was found and then skip the scan),
so I'll skip those, which might also help by substantially reducing
the work being done on both HDD and NVME/SSD.

I might also look into avoiding the scan completely if there are only
few files list files to reload (instead of say the entire lot), but
I'm not sure how much that will help once/if it is only done for the
few packages that need a reload (if the current caching is not effective
for some reason, then I'd assume it might not help a lot?). And thinking about it perhaps even the force option is not really necessary after
the scan fix anyway? (But it would still help in the apt splitting case mentioned above though.)

I'm attaching the change to skip unnecessary optimization scan, in
case you want to give it a go, although I've only compile tested it
for now.

PS: it might also be a good idea to reconsider the whole scan - fiemap often creates just as many seeks as a stat or an open, especially on more complicated filesystems. In fact, sorting by inode numbers (which are essentially free info) is often a good approximation for locality on many filesystems and might be a good replacement strategy.

Perhaps, I would need to check the context from when it was added as
I cannot recall whether this option was considered and discarded,
and otherwise it would need to be benchmarked on a bunch of different filesystems and types of disks to see whether there would be
significant performance degradation/regression.

While testing this briefly, I noticed that on my system the main
program emitting these ioctls was actually man-db from its trigger,
which has code based on the dpkg logic. So that would also need
additional changes to support this, perhaps the trigger could honor
the DPKG_FORCE envvar and pass some new option to man-db? (Colin?)

Thanks,
Guillem

--- SoupGate-Win32 v1.05
* Origin: you cannot sedate... all the things you hate (1:229/2)