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
