diff mbox series

[bug#67238] derivations: Avoid readlink syscalls in read-derivation-from-file.

Message ID a0061470c9d839f1503fea238c5e3b3ed92fd660.1700220934.git.mail@cbaines.net
State New
Headers
Series [bug#67238] derivations: Avoid readlink syscalls in read-derivation-from-file. |

Commit Message

Christopher Baines Nov. 17, 2023, 11:35 a.m. UTC 
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
---
 guix/derivations.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)


base-commit: e35b7c5386c1bfacf47ed31bac9b503373dd26fc

Comments

Ludovic Courtès Nov. 24, 2023, 11:28 a.m. UTC | #1 
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’.
Christopher Baines Nov. 25, 2023, 8:58 p.m. UTC | #2 
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 mbox series

Patch

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)))