| Message ID | a0061470c9d839f1503fea238c5e3b3ed92fd660.1700220934.git.mail@cbaines.net |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#67238] derivations: Avoid readlink syscalls in read-derivation-from-file. | expand |
Hi, Christopher Baines <mail@cbaines.net> skribis: > strace -c reports over 10,000 readlink syscalls when reading the derivation > for the hello package. By just setting the %file-port-name-canonicalization > fluid, this drops to less than 10. > > I'm not sure if this actually improves performance, but doing less is surely > better. > > * guix/derivations.scm (read-derivation-from-file): Set > %file-port-name-canonicalization to 'none when calling call-with-input-file. > > Change-Id: I1ff16a059160576a576f2e9ed881379596e66af3 [...] > + (let ((drv > + ;; Avoid calling scm_i_relativize_path in > + ;; fport_canonicalize_filename since this leads to lots of > + ;; readlink calls > + (with-fluids ((%file-port-name-canonicalization 'none)) > + (call-with-input-file file read-derivation)))) This is already done in ‘run-guix’ in (guix ui), for all the ‘guix’ commands (so this patch would be a slight performance regression for Guix itself). I’d suggest setting this fluid globally in applications that use Guix (the Build Coordinator, etc.), as is done in Guix itself. WDYT? Ludo’.
Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > Christopher Baines <mail@cbaines.net> skribis: > >> strace -c reports over 10,000 readlink syscalls when reading the derivation >> for the hello package. By just setting the %file-port-name-canonicalization >> fluid, this drops to less than 10. >> >> I'm not sure if this actually improves performance, but doing less is surely >> better. >> >> * guix/derivations.scm (read-derivation-from-file): Set >> %file-port-name-canonicalization to 'none when calling call-with-input-file. >> >> Change-Id: I1ff16a059160576a576f2e9ed881379596e66af3 > > [...] > >> + (let ((drv >> + ;; Avoid calling scm_i_relativize_path in >> + ;; fport_canonicalize_filename since this leads to lots of >> + ;; readlink calls >> + (with-fluids ((%file-port-name-canonicalization 'none)) >> + (call-with-input-file file read-derivation)))) > > This is already done in ‘run-guix’ in (guix ui), for all the ‘guix’ > commands (so this patch would be a slight performance regression for > Guix itself). > > I’d suggest setting this fluid globally in applications that use Guix > (the Build Coordinator, etc.), as is done in Guix itself. > > WDYT? Ah, I didn't realise it was already set for Guix scripts. But yeah, setting it in other places that read derivations makes sense.
diff --git a/guix/derivations.scm b/guix/derivations.scm index 9fec7f4f0b..e6ecb570c4 100644 --- a/guix/derivations.scm +++ b/guix/derivations.scm @@ -556,7 +556,12 @@ (define (read-derivation-from-file file) ;; and because the same argument is read more than 15 times on average ;; during something like (package-derivation s gdb). (or (and file (hash-ref %derivation-cache file)) - (let ((drv (call-with-input-file file read-derivation))) + (let ((drv + ;; Avoid calling scm_i_relativize_path in + ;; fport_canonicalize_filename since this leads to lots of + ;; readlink calls + (with-fluids ((%file-port-name-canonicalization 'none)) + (call-with-input-file file read-derivation)))) (hash-set! %derivation-cache file drv) drv)))