diff mbox series

[bug#49421] profiles: Optimise 'fonts-dir-file'.

Message ID e1af6b02effdeaf4a05f1ddb602003d57961ec62.camel@telenet.be
State New
Headers show
Series [bug#49421] profiles: Optimise 'fonts-dir-file'. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Maxime Devos July 5, 2021, 7:06 p.m. UTC
Hi guix,

These two patches should speed up profile generation
by optimising the 'fonts-dir-file' hook.  The first patch
is the most important; from the commit message:

‘Only let the build G-exp refer to inputs that might actually
have fonts.  That way, if the list of fonts in the manifest
didn't change and the new manifest is built, the font files
will not be rebuilt.’

The second patch doesn't bring much.

To test, you can test whether things like ...

$ ./pre-inst-env guix environment --ad-hoc --pure lagrange -- lagrange

still work (lagrange is a graphical application using fonts).
Not sure what a good ‘benchmark’ would be.

Unfortunately, this does not help with the "guix package -i" case,
as in that case, the code doesn't have access to all the package objects,
and will have to satisfy itself with the store paths, in which case the
profile code pessimistically assumes the store item has fonts ...

I suppose this restriction could be lifted if/when the gs-fonts package
is renamed to font-ghostscript or something like that ...

However, the optimisation should work in the "guix environment ... --ad-hoc ...",
"guix system reconfigure ..." and (I'd presume) the guix home-manager cases,
though I only tested the first.

Greetings,
Maxime.

Comments

Ludovic Courtès July 7, 2021, 4:52 p.m. UTC | #1
Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> These two patches should speed up profile generation
> by optimising the 'fonts-dir-file' hook.  The first patch
> is the most important; from the commit message:
>
> ‘Only let the build G-exp refer to inputs that might actually
> have fonts.  That way, if the list of fonts in the manifest
> didn't change and the new manifest is built, the font files
> will not be rebuilt.’
>
> The second patch doesn't bring much.
>
> To test, you can test whether things like ...
>
> $ ./pre-inst-env guix environment --ad-hoc --pure lagrange -- lagrange
>
> still work (lagrange is a graphical application using fonts).
> Not sure what a good ‘benchmark’ would be.
>
> Unfortunately, this does not help with the "guix package -i" case,
> as in that case, the code doesn't have access to all the package objects,
> and will have to satisfy itself with the store paths, in which case the
> profile code pessimistically assumes the store item has fonts ...
>
> I suppose this restriction could be lifted if/when the gs-fonts package
> is renamed to font-ghostscript or something like that ...

Do you know what’s taking time in the ‘fonts-dir-file’ hook?
(On my x86_64 laptop with SSD, it runs in 4.4s on a cold cache for a
profile containing 14 font packages, and in 1.2s on a warm cache.)

IIUC, the hook completes almost instantaneously when creating a profile
without fonts because (null? fonts-dirs) is true.

In other cases, one option to speed things up (and possibly remove the
hook’s dependency on mkfontdir) would be to create ‘fonts.dir’ and
‘fonts.scale’ in a build phase of each font package, *provided* there’s
a cheap way to assemble several such files, such as mere concatenation.
That way, the profile hook would only need to assemble these files as
opposed to traversing all the font files.

Given that ‘fonts.dir’ and ‘fonts.scale’ are mere text files with one
line per font, this approach looks doable.

WDYT?

> From 4fe1e30e33c01be9fd17cf240732b3351c7b0fa4 Mon Sep 17 00:00:00 2001
> From: Maxime Devos <maximedevos@telenet.be>
> Date: Mon, 5 Jul 2021 18:55:31 +0200
> Subject: [PATCH 1/2] profiles: Optimise 'fonts-dir-file'.
>
> Only let the build G-exp refer to inputs that might actually
> have fonts.  That way, if the list of fonts in the manifest
> didn't change and the new manifest is built, the font files
> will not be rebuilt.
>
> * guix/profiles.scm
>   (fonts-dir-file)[has-fonts?]: New predicate.
>   (fonts-dir-file)[relevant-inputs]: New variable.
>   (fonts-dir-file)[build]: Use 'relevant-inputs' instead of
>   'manifest-inputs'.
> * doc/contributing.texi (Fonts): Note the 'fonts-' naming
>   convention is technically important now.
> * gnu/packages/ghostscript.scm (gs-fonts): Work-around the
>   package name contravening the convention.

To me this approach should be a last resort because it relies on
conventions and manual annotations, and it’s easy to forget these and
get incorrect results (we could end up having to deal with reports like
“I installed font X, why doesn’t in show up in the font list?”).

> +            ;; In the upstream 'guix' channel, font packages should
> +            ;; be named font-SOMETHING.  But if another channel
> +            ;; names its fonts differently but uses font-build-system,
> +            ;; accepting that seems friendly.
> +            (eq? 'font (build-system-name (package-build-system thing)))

In general ‘build-system-name’ shouldn’t be used; the name is meant to
be a debugging aid.

Thanks,
Ludo’.
Maxime Devos July 15, 2021, 4:28 p.m. UTC | #2
Ludovic Courtès schreef op wo 07-07-2021 om 18:52 [+0200]:
> Hi,
> 
> Maxime Devos <maximedevos@telenet.be> skribis:
> 
> > These two patches should speed up profile generation
> > by optimising the 'fonts-dir-file' hook.  The first patch
> > is the most important; from the commit message:
> > 
> > ‘Only let the build G-exp refer to inputs that might actually
> > have fonts.  That way, if the list of fonts in the manifest
> > didn't change and the new manifest is built, the font files
> > will not be rebuilt.’
> > 
> > The second patch doesn't bring much.
> > 
> > To test, you can test whether things like ...
> > 
> > $ ./pre-inst-env guix environment --ad-hoc --pure lagrange -- lagrange
> > 
> > still work (lagrange is a graphical application using fonts).
> > Not sure what a good ‘benchmark’ would be.
> > 
> > Unfortunately, this does not help with the "guix package -i" case,
> > as in that case, the code doesn't have access to all the package objects,
> > and will have to satisfy itself with the store paths, in which case the
> > profile code pessimistically assumes the store item has fonts ...
> > 
> > I suppose this restriction could be lifted if/when the gs-fonts package
> > is renamed to font-ghostscript or something like that ...
> 
> Do you know what’s taking time in the ‘fonts-dir-file’ hook?
> (On my x86_64 laptop with SSD, it runs in 4.4s on a cold cache for a
> profile containing 14 font packages, and in 1.2s on a warm cache.)

I noticed ‘building fonts directories’ was taking long (9 minutes or so,
when using "guix package -r two packages that don't have fonts"),
so I took the opportunity to "strace" the process, though I was a little late.
Some statistics:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 27,14    1,676551         117     14297           rename
 19,60    1,211012          13     90829         8 lstat
 18,34    1,133182          63     17787        15 statx
 11,80    0,729185          51     14297           link
  6,65    0,411065       68510         6           wait4
  5,08    0,313632          10     28651           chmod
  3,66    0,226258          14     15114           readlink
  2,18    0,134913           8     15193           utimensat
  1,78    0,110257          32      3431           rmdir
  0,91    0,056504           7      7370           getdents64
  0,82    0,050817           3     14295           getpid
  0,47    0,029074           7      3736           openat
  0,46    0,028547           8      3479      3428 unlink
  0,36    0,021940           5      3747           close
  0,26    0,016355           4      3704           fstat
  0,16    0,009752          10       898           lchown
  0,12    0,007242           4      1800           geteuid
  0,07    0,004203          62        67           read
  0,06    0,003512           3       900           getegid
  0,03    0,001959         326         6           clone
  0,02    0,001133          18        62           write
  0,01    0,000488           3       127         1 fcntl
  0,01    0,000451           3       136           lseek
  0,00    0,000197          14        14           mkdir
  0,00    0,000065          13         5           select
  0,00    0,000045           5         8           chown
  0,00    0,000044           7         6           brk
  0,00    0,000027          13         2           munmap
  0,00    0,000023          11         2           pipe
  0,00    0,000013           6         2           stat
  0,00    0,000007           7         1           futex
  0,00    0,000004           2         2           getuid
  0,00    0,000000           0         1           rt_sigreturn
------ ----------- ----------- --------- --------- ----------------
100,00    6,178457          25    239975      3452 total

And a part of the log:

<start of log>
lstat("/gnu/store/.links/[...]2i", {st_mode=S_IFLNK|0777, st_size=124, ...}) = 0
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", {st_mode=S_IFDIR|0555, st_size=4112, ...>
chmod("/gnu/store/[..]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", 040755) = 0
getpid()                                = 26371
link("/gnu/store/.links/[..]2i", "/gnu/store/.tmp-link-26371-632297102") = 0
rename("/gnu/store/.tmp-link-26371-632297102", "/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/2>
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", {st_mode=S_IFDIR|0755, st_size=4112, ...>
chmod("/gnu/store/[...]-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", 040555) = 0
utimensat(AT_FDCWD, "/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", [{tv_sec=1626363034, tv_ns>
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/2223.png", {st_mode=S_IFLNK|0777, st_size=>
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/2223.png", {st_mode=S_IFLNK|0777, st_size=>
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/2223.png", {st_mode=S_IFLNK|0777, st_size=>
readlink("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/2223.png", "/gnu/store/bf582s56ldb3y34p>
statx(AT_FDCWD, "/gnu/store/.links/[...]v3", AT_STATX_SYNC_AS_STAT|AT_SYMLINK_NOFOLLOW, 0, {stx_mask=STATX_ALL|>
lstat("/gnu/store/.links/[...]v3", {st_mode=S_IFLNK|0777, st_size=124, ...}) = 0
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", {st_mode=S_IFDIR|0555, st_size=4112, ...>
chmod("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", 040755) = 0
getpid()                                = 26371
link("/gnu/store/.links/[...]v3", "/gnu/store/.tmp-link-26371-114005693") = 0
rename("/gnu/store/.tmp-link-26371-114005693", "/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/2>
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", {st_mode=S_IFDIR|0755, st_size=4112, ...>
chmod("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", 040555) = 0
utimensat(AT_FDCWD, "/gnu/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", [{tv_sec=1626363034, tv_ns>
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/22CC.png", {st_mode=S_IFLNK|0777, st_size=>
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/22CC.png", {st_mode=S_IFLNK|0777, st_size=>
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/22CC.png", {st_mode=S_IFLNK|0777, st_size=>
readlink("/gnu/store/h6681qq9s5kqjvdzkhqhddqbjj3q48nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283/22CC.png", "/gnu/store/bf582s56ldb3y34p>
statx(AT_FDCWD, "/gnu/store/.links/[...]my", AT_STATX_SYNC_AS_STAT|AT_SYMLINK_NOFOLLOW, 0, {stx_mask=STATX_ALL|>
lstat("/gnu/store/.links/[...]my", {st_mode=S_IFLNK|0777, st_size=124, ...}) = 0
lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", {st_mode=S_IFDIR|0555, st_size=4112, ...>
chmod("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", 040755) = 0
getpid()                                = 26371
<end of log>

It seems like ‘duplication’ is taking a lot of time!
(See nix/libstore/optimise-store.cc and (guix store deduplication).)

> IIUC, the hook completes almost instantaneously when creating a profile
> without fonts because (null? fonts-dirs) is true.
> 
> In other cases, one option to speed things up (and possibly remove the
> hook’s dependency on mkfontdir) would be to create ‘fonts.dir’ and
> ‘fonts.scale’ in a build phase of each font package, *provided* there’s
> a cheap way to assemble several such files, such as mere concatenation.
> That way, the profile hook would only need to assemble these files as
> opposed to traversing all the font files.

Looking at ~/.guix-profile/share/fonts/truetype/fonts.dir,
it seems to be simply a list of entries lke

   DejaVuSans-Bold.ttf -misc-dejavu sans-bold-r-normal--0-0-0-0-p-0-koi8-ru

with additionally as first line the number of fonts.  It appears to be
sorted by font file name (DejaVuSans-Bold.ttf) though, though that only
seems to be to not depend on file system ordering.  Not quite _mere_
concatenation, but should be easy enough, I think.

(Likewise for fonts.scaale)

I would need to look at the code of "mkfontscale" and "mkfontdir" to be sure
though.

> Given that ‘fonts.dir’ and ‘fonts.scale’ are mere text files with one
> line per font, this approach looks doable.
> 
> WDYT?

That seems a good idea, reducing the amount of 'stat'ing, 'readdir'ing,
and 'read'ing (mkfontscale needs to actually read the font files)
which should help on spinning disks (and other disks, but on SSDs it's probably
less noticable).

> > From 4fe1e30e33c01be9fd17cf240732b3351c7b0fa4 Mon Sep 17 00:00:00 2001
> > From: Maxime Devos <maximedevos@telenet.be>
> > Date: Mon, 5 Jul 2021 18:55:31 +0200
> > Subject: [PATCH 1/2] profiles: Optimise 'fonts-dir-file'.
> > 
> > Only let the build G-exp refer to inputs that might actually
> > have fonts.  That way, if the list of fonts in the manifest
> > didn't change and the new manifest is built, the font files
> > will not be rebuilt.
> > 
> > * guix/profiles.scm
> >   (fonts-dir-file)[has-fonts?]: New predicate.
> >   (fonts-dir-file)[relevant-inputs]: New variable.
> >   (fonts-dir-file)[build]: Use 'relevant-inputs' instead of
> >   'manifest-inputs'.
> > * doc/contributing.texi (Fonts): Note the 'fonts-' naming
> >   convention is technically important now.
> > * gnu/packages/ghostscript.scm (gs-fonts): Work-around the
> >   package name contravening the convention.
> 
> To me this approach should be a last resort because it relies on
> conventions and manual annotations, and it’s easy to forget these and
> get incorrect results (we could end up having to deal with reports like
> “I installed font X, why doesn’t in show up in the font list?”).

AFAIK, all font packages in Guix (except gs-fonts) are currently
correctly-named.  They aren't going to spontanuously rename theirselves.
So to me, the only potential problem seems to be _new_ font packages
ignoring the conventions.  But new packages should always be reviewed and
tested, so such reports don't seem plausible to me.

(Also, the manual annotation would go away once 'gs-fonts' is renamed.
I'll run "echo /gnu/store/*/share/fonts > atchie-fonts" though to see
if there are other packages not following the convention.)

> > +            ;; In the upstream 'guix' channel, font packages should
> > +            ;; be named font-SOMETHING.  But if another channel
> > +            ;; names its fonts differently but uses font-build-system,
> > +            ;; accepting that seems friendly.
> > +            (eq? 'font (build-system-name (package-build-system thing)))
> 
> In general ‘build-system-name’ shouldn’t be used; the name is meant to
> be a debugging aid.

I could do (eq? font-build-system (packag-build-system thing)) instead
or drop this clause.

> Thanks,
> Ludo’.
Maxime Devos July 15, 2021, 6:48 p.m. UTC | #3
> (Also, the manual annotation would go away once 'gs-fonts' is renamed.
> I'll run "echo /gnu/store/*/share/fonts > atchie-fonts" though to see
> if there are other packages not following the convention.)

Scheme code:

(use-modules (ice-9 ftw) (srfi srfi-1))
(define (has-share/fonts? o)
  (file-exists? (string-append "/gnu/store/" o "/share/fonts")))
(define t (scandir "/gnu/store" has-share/fonts?))
(define t2 (map (lambda (x) (substring x 33)) t))
(define t3 (delete-duplicates t2))
(define (fonty? x) (string-prefix? "font-" x))
(partition (lambda (x) (string-prefix? "font-" x)) t3)

Output on my system:

$1 = ("font-alias-1.0.3" [...])
$2 = ("fonts-dir" "profile" "mate-1.24.1" "denemo-2.5.0" "mate-1.24.0" "gs-fonts-8.11")

"fonts-dir", "profile" and "gs-fonts" are expected.

"mate" uses "union-build" and has "font-cantarell" in 'inputs'.
Apparently "denemo" has some font files as well.
Something will need to be figured out for these cases,
if the approach of my patch is followed.

I think it's better to optimise 'union-build' and the store deduplication
code though (maybe subdirectories can be process concurrently, to maximise I/O
utilisation?). That should improve performance outside the 'fonts-dir-file' hook
as well.

Greetings,
Maxime.
Ludovic Courtès July 16, 2021, 4:05 p.m. UTC | #4
Hi!

Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op wo 07-07-2021 om 18:52 [+0200]:

[...]

>> Do you know what’s taking time in the ‘fonts-dir-file’ hook?
>> (On my x86_64 laptop with SSD, it runs in 4.4s on a cold cache for a
>> profile containing 14 font packages, and in 1.2s on a warm cache.)
>
> I noticed ‘building fonts directories’ was taking long (9 minutes or so,
> when using "guix package -r two packages that don't have fonts"),

Ouch, that’s way more than what I’m observing.  :-)  Do you have a
manifest or package list to reproduce that?  Were those timings on a
spinning disk?

> so I took the opportunity to "strace" the process, though I was a little late.
> Some statistics:
>
> % time     seconds  usecs/call     calls    errors syscall
> ------ ----------- ----------- --------- --------- ----------------
>  27,14    1,676551         117     14297           rename
>  19,60    1,211012          13     90829         8 lstat
>  18,34    1,133182          63     17787        15 statx
>  11,80    0,729185          51     14297           link

[...]

> statx(AT_FDCWD, "/gnu/store/.links/[...]my", AT_STATX_SYNC_AS_STAT|AT_SYMLINK_NOFOLLOW, 0, {stx_mask=STATX_ALL|>
> lstat("/gnu/store/.links/[...]my", {st_mode=S_IFLNK|0777, st_size=124, ...}) = 0
> lstat("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", {st_mode=S_IFDIR|0555, st_size=4112, ...>
> chmod("/gnu/store/[...]nm-fonts-dir/share/fonts/mathjax/HTML-CSS/TeX/png/AMS/Regular/283", 040755) = 0
> getpid()                                = 26371
> <end of log>
>
> It seems like ‘duplication’ is taking a lot of time!

It surely does but we need more data before we can draw a conclusion
(the system time above accounts for ~6 seconds, not 9 minutes).

Anyway, it looks like we should be able to push most of the I/O work to
packages.  Let’s see!

Thanks,
Ludo’.
Maxime Devos July 17, 2021, 2:42 p.m. UTC | #5
Ludovic Courtès schreef op vr 16-07-2021 om 18:05 [+0200]:
> Ouch, that’s way more than what I’m observing.  :-)  Do you have a
> manifest or package list to reproduce that?  Were those timings on a
> spinning disk?

It is on a spinning disk.  I attached the output of "guix package --export-manifest".

Greetings,
Maxime.
;; Dit ‘manifest’ kan worden doorgegeven aan ‘guix package -m’ om de inhoud
;; van je profiel te reproduceren.  Dit is ‘symbolisch’: het verwijst enkel
;; naar de pakketnamen.  Om precies hetzelfde profiel te reproduceren, moet
;; je ook de lijst van gebruikte kanalen bewaren, zoals teruggegeven door ‘guix describe’.
;; Zie de sectie ‘Guix reproduceren’ in de handleiding.

(specifications->manifest
  (list "jami-gnome"
        "texmacs"
        "git:send-email"
        "lagrange"
        "vlc"
        "blueman"
        "bitlbee-purple"
        "network-manager-openconnect"
        "audacity"
        "evolution"
        "kiwix-desktop"
        "icecat"
        "simple-scan"
        "guile-studio"
        "ungoogled-chromium"
        "pidgin-otr"
        "emacs-calibredb"
        "calibre"
        "btrfs-progs"
        "tiled"
        "cheese"
        "seahorse"
        "wireshark"
        "git"
        "reuse"
        "gzochi"
        "zbar"
        "texmaker"
        "gnumeric"
        "gpa"
        "texlive-base"
        "gimp"
        "pinentry"
        "emacs-exwm"
        "geany"
        "sky"
        "texlive"
        "gnurobots"
        "graphviz"
        "guile-sdl"
        "guile-chickadee"
        "emacs"
        "emacs-magit-org-todos-el"
        "emacs-magit-annex"
        "emacs-magit-gerrit"
        "emacs-magit"
        "tome4"
        "emacs-guix"
        "zile-on-guile"
        "emacs-debbugs"
        "emacs-helm-bibtex"
        "emacs-ebdb"
        "emacs-helm-firefox"
        "emacs-helm-eww"
        "emacs-helm"
        "emacs-helm-wikipedia"
        "man-pages"
        "emacs-nov-el"
        "gnupg"
        "rr"
        "openssh"
        "irssi"
        "dee"
        "emacs-bluetooth"
        "guile-cairo"
        "xdg-dbus-proxy"
        "fetchmail"
        "libimobiledevice"
        "emacs-ledger-mode"
        "gdb"
        "valgrind"
        "plotutils"
        "xdg-utils"
        "artanis"
        "ghostscript"
        "dbus:doc"
        "dbus"
        "haunt"
        "nethack"
        "gcc-toolchain"
        "emacs-company-coq"
        "emacs-flycheck"
        "emacs-flycheck-guile"
        "emacs-ghub"
        "emacs-build-farm"
        "emacs-ebib"
        "emacs-company-math"
        "cryptsetup"
        "emacs-paredit"
        "emacs-ac-geiser"
        "emacs-geiser"
        "emacs-gcmh"
        "emacs-julia-mode"
        "emacs-yasnippet"
        "emacs-ox-epub"
        "emacs-bongo"
        "emacs-elf-mode"
        "emacs-js2-mode"
        "emacs-company-ebdb"
        "emacs-bbdb"
        "emacs-ffap-rfc-space"
        "z3"
        "texlive-latex-babel"
        "texlive-latex-geometry"
        "texlive-latex-hyperref"
        "texlive-latex-pgf"
        "texlive-latex-graphics"
        "glibc:debug"
        "glibc"
        "guile:debug"
        "guile"
        "aspell"
        "aspell-dict-de"
        "aspell-dict-nl"
        "aspell-dict-fr"
        "aspell-dict-en"
        "xxd"
        "go-ipfs"
        "unzip"
        "screen"
        "libreoffice"
        "htop"
        "scsh"
        "guile-miniadapton"
        "sicp"
        "mig"
        "texlive-epsf"
        "font-mathjax"
        "guile-ac-d-bus"
        "gash"
        "zip"
        "jacal"
        "gambit-c"
        "strace"
        "font-dejavu"
        "guile-fibers"
        "pkg-config"
        "ncurses"))
Ludovic Courtès July 21, 2021, 2:48 p.m. UTC | #6
Hello,

Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op vr 16-07-2021 om 18:05 [+0200]:
>> Ouch, that’s way more than what I’m observing.  :-)  Do you have a
>> manifest or package list to reproduce that?  Were those timings on a
>> spinning disk?
>
> It is on a spinning disk.  I attached the output of "guix package --export-manifest".

I isolated ‘fonts-dir.drv’ for that profile and here’s what I get with a
~5yo SSD:

--8<---------------cut here---------------start------------->8---
$ time guix build  /gnu/store/nmr6clkx4n554a0gynr6sb4a43dj6mfa-fonts-dir.drv --check
The following profile hook will be built:
   /gnu/store/nmr6clkx4n554a0gynr6sb4a43dj6mfa-fonts-dir.drv
building fonts directory...
successfully built /gnu/store/nmr6clkx4n554a0gynr6sb4a43dj6mfa-fonts-dir.drv
successfully built /gnu/store/nmr6clkx4n554a0gynr6sb4a43dj6mfa-fonts-dir.drv
/gnu/store/fgjmwpgrmp9m83qxvqrs1ghwasjrjxpc-fonts-dir

real    0m24.651s
user    0m2.120s
sys     0m0.164s
$ time guix build  /gnu/store/nmr6clkx4n554a0gynr6sb4a43dj6mfa-fonts-dir.drv --check
The following profile hook will be built:
   /gnu/store/nmr6clkx4n554a0gynr6sb4a43dj6mfa-fonts-dir.drv
building fonts directory...
successfully built /gnu/store/nmr6clkx4n554a0gynr6sb4a43dj6mfa-fonts-dir.drv
successfully built /gnu/store/nmr6clkx4n554a0gynr6sb4a43dj6mfa-fonts-dir.drv
/gnu/store/fgjmwpgrmp9m83qxvqrs1ghwasjrjxpc-fonts-dir

real    0m12.025s
user    0m1.924s
sys     0m0.036s
--8<---------------cut here---------------end--------------->8---

That’s fairly significant (it baffles me that it’s still 20x less than
what you see with a spinning disk!).

There are only 327 fonts in there:

--8<---------------cut here---------------start------------->8---
$ wc -l /gnu/store/fgjmwpgrmp9m83qxvqrs1ghwasjrjxpc-fonts-dir/share/fonts/truetype/fonts.*
  328 /gnu/store/fgjmwpgrmp9m83qxvqrs1ghwasjrjxpc-fonts-dir/share/fonts/truetype/fonts.dir
  328 /gnu/store/fgjmwpgrmp9m83qxvqrs1ghwasjrjxpc-fonts-dir/share/fonts/truetype/fonts.scale
  656 totalo
--8<---------------cut here---------------end--------------->8---

Thanks,
Ludo’.
Ludovic Courtès July 21, 2021, 2:58 p.m. UTC | #7
Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> I think it's better to optimise 'union-build' and the store deduplication
> code though (maybe subdirectories can be process concurrently, to maximise I/O
> utilisation?). That should improve performance outside the 'fonts-dir-file' hook
> as well.

I’m afraid there’s little room for improvement on ‘union-build’ (Mark
put quite some effort on it back in commit
12129998689648923b58c426362a1bc875da75f9), though with a fresh eye
perhaps you’ll find optimization opportunities.

As for deduplication, there are two cases: deduplication for
substitutes, and deduplication upon build completion (what we see
here).

In the former case, deduplication is part of the substitute unpacking
pipeline (see
<https://guix.gnu.org/en/blog/2021/getting-bytes-to-disk-more-quickly/>)
and I think syscalls are reduced to the minimum necessary (that’s (guix
store deduplication)).

The strace snippet you sent suggests that, in the latter case, the
daemon’s code stats the same file several times, so maybe there’s room
for improvement here.

Thanks,
Ludo’.
diff mbox series

Patch

From a706b8be4f54530b1cd12c03a1bf3941be43be3c Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Mon, 5 Jul 2021 20:16:22 +0200
Subject: [PATCH 2/2] profiles: Avoid dependency on 'mkfontdir' and friend when
 unused.

Avoid depending on on "mkfontdir" and "mkfontscale" if
they won't actually be used, to avoid building the
aforementioned packages when they are updated.

* guix/profiles.scm
  (fonts-dir-file)[build]: Note why the '(null? fonts-dir)' check
  cannot be removed.
  (fonts-dir-file): When the 'relevant-inputs' list is empty,
  just build an empty directory.
---
 guix/profiles.scm | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 0f9df68f42..6cf480ddf2 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1544,6 +1544,8 @@  files for the fonts of the @var{manifest} entries."
                                             "/share/fonts")
                                        '#$relevant-inputs))))
           (mkdir #$output)
+          ;; has-fonts? can have false positives,
+          ;; so this check is necessary.
           (if (null? fonts-dirs)
               (exit #t)
               (let* ((share-dir   (string-append #$output "/share"))
@@ -1585,15 +1587,20 @@  files for the fonts of the @var{manifest} entries."
                                   (delete-file fonts-dir-file))))
                             directories)))))))
 
-  (gexp->derivation "fonts-dir" build
-                    #:modules '((guix build utils)
-                                (guix build union)
-                                (srfi srfi-26))
-                    #:local-build? #t
-                    #:substitutable? #f
-                    #:properties
-                    `((type . profile-hook)
-                      (hook . fonts-dir))))
+  (if (null? relevant-inputs)
+      ;; Avoid depending on on "mkfontdir" and "mkfontscale" if
+      ;; they won't actually be used, to avoid building the aforementioned
+      ;; packages when they are updated.
+      (lower-object (file-union "fonts-dir" '()))
+      (gexp->derivation "fonts-dir" build
+                        #:modules '((guix build utils)
+                                    (guix build union)
+                                    (srfi srfi-26))
+                        #:local-build? #t
+                        #:substitutable? #f
+                        #:properties
+                        `((type . profile-hook)
+                          (hook . fonts-dir)))))
 
 (define (manual-database manifest)
   "Return a derivation that builds the manual page database (\"mandb\") for
-- 
2.32.0