diff mbox series

[bug#68266,1/7] gnu: Memozise make-ld-wrapper results.

Message ID 87plyaw1bn.fsf@gnu.org
State New
Headers show
Series [bug#68266,1/7] gnu: Memozise make-ld-wrapper results. | expand

Commit Message

Ludovic Courtès Jan. 9, 2024, 11:10 p.m. UTC
Hi,

Christopher Baines <mail@cbaines.net> skribis:

> I believe the reason packages from make-ld-wrapper were showing up
> multiple times in the cache for me is linked to it's use in the
> cross-base module, as part of the cross-gcc procedure.
>
> A later commit does change cross-gcc to return a single package record
> for some given arguments, so that probably resolves the biggest misuse
> of make-ld-wrapper.

Oh, I see.

Before resorting to memoization (which doesn’t come for free), we can
for instance make sure we do not create new package records from inputs
fields (since they are thunked, a fresh record would be created each
time the field is accessed).  Here’s an example:
This particular one doesn’t have a huge impact on simple cases but it
might pay off for bigger graphs:

--8<---------------cut here---------------start------------->8---
$ GUIX_PROFILING=object-cache ./pre-inst-env guix build hello --target=aarch64-linux-gnu --no-grafts -d
/gnu/store/i8x1gg73x3fvxn4lrrgbcb97sz4qq1yx-hello-2.12.1.drv
Object Cache:
  fresh caches:    20
  lookups:       5514
  hits:          5008 (90.8%)
  cache size:     505 entries
$ # before:
$ GUIX_PROFILING=object-cache ./pre-inst-env guix build hello --target=aarch64-linux-gnu --no-grafts -d
/gnu/store/kdnh9bwg874aq8bvvw9y2vkl7z94icqa-hello-2.12.1.drv
Object Cache:
  fresh caches:    20
  lookups:       5406
  hits:          4907 (90.8%)
  cache size:     498 entries
--8<---------------cut here---------------end--------------->8---

> I think there's other cases (in the llvm and mold modules) where it
> looks like it's called multiple times with the same arguments, so maybe
> that's an argument for having memoization around make-ld-wrapper even
> though it's not needed for all uses.

On guix-devel, Efraim mentioned cargo-build-system packages.  Is that
related?

Thanks for looking into this!

Ludo’.

Comments

Christopher Baines Jan. 10, 2024, 12:28 p.m. UTC | #1
Ludovic Courtès <ludo@gnu.org> writes:

> Christopher Baines <mail@cbaines.net> skribis:
>
>> I believe the reason packages from make-ld-wrapper were showing up
>> multiple times in the cache for me is linked to it's use in the
>> cross-base module, as part of the cross-gcc procedure.
>>
>> A later commit does change cross-gcc to return a single package record
>> for some given arguments, so that probably resolves the biggest misuse
>> of make-ld-wrapper.
>
> Oh, I see.
>
> Before resorting to memoization (which doesn’t come for free), we can
> for instance make sure we do not create new package records from inputs
> fields (since they are thunked, a fresh record would be created each
> time the field is accessed).  Here’s an example:
>
> diff --git a/gnu/packages/cross-base.scm b/gnu/packages/cross-base.scm
> index 6ee7b315d8..ca3b41a4c7 100644
> --- a/gnu/packages/cross-base.scm
> +++ b/gnu/packages/cross-base.scm
> @@ -306,6 +306,11 @@ (define* (cross-gcc target
>  XGCC as the base compiler.  Use XBINUTILS as the associated cross-Binutils.
>  If LIBC is false, then build a GCC that does not target a libc; otherwise,
>  target that libc."
> +  (define ld-wrapper
> +    (make-ld-wrapper (string-append "ld-wrapper-" target)
> +                     #:target (const target)
> +                     #:binutils xbinutils))
> +
>    (package
>      (inherit xgcc)
>      (name (string-append "gcc-cross-"
> @@ -313,8 +318,7 @@ (define* (cross-gcc target
>                           target))
>      (source
>       (origin
> -       (inherit
> -        (package-source xgcc))
> +       (inherit (package-source xgcc))
>         (patches
>          (append
>           (origin-patches (package-source xgcc))
> @@ -353,10 +357,7 @@ (define* (cross-gcc target
>         ,@(cross-gcc-arguments target xgcc libc)))
>  
>      (native-inputs
> -     `(("ld-wrapper-cross" ,(make-ld-wrapper
> -                             (string-append "ld-wrapper-" target)
> -                             #:target (const target)
> -                             #:binutils xbinutils))
> +     `(("ld-wrapper-cross" ,ld-wrapper)
>         ("binutils-cross" ,xbinutils)
>  
>         ,@(let ((inputs (append (package-inputs xgcc)
>
>
> This particular one doesn’t have a huge impact on simple cases but it
> might pay off for bigger graphs:
>
> $ GUIX_PROFILING=object-cache ./pre-inst-env guix build hello --target=aarch64-linux-gnu --no-grafts -d
> /gnu/store/i8x1gg73x3fvxn4lrrgbcb97sz4qq1yx-hello-2.12.1.drv
> Object Cache:
>   fresh caches:    20
>   lookups:       5514
>   hits:          5008 (90.8%)
>   cache size:     505 entries
> $ # before:
> $ GUIX_PROFILING=object-cache ./pre-inst-env guix build hello --target=aarch64-linux-gnu --no-grafts -d
> /gnu/store/kdnh9bwg874aq8bvvw9y2vkl7z94icqa-hello-2.12.1.drv
> Object Cache:
>   fresh caches:    20
>   lookups:       5406
>   hits:          4907 (90.8%)
>   cache size:     498 entries
>
>> I think there's other cases (in the llvm and mold modules) where it
>> looks like it's called multiple times with the same arguments, so maybe
>> that's an argument for having memoization around make-ld-wrapper even
>> though it's not needed for all uses.
>
> On guix-devel, Efraim mentioned cargo-build-system packages.  Is that
> related?

Looking at the various changes, while rust-sysroot doesn't appear that
many times in the cache, I think it's responsible for most of the
slowness. Maybe that's due in part to what you say above, using the
cross- procedures in the native-inputs field.

Maybe the transformation you suggest above could be applied, but I'd be
worried about how that would work if you set one
%current-system/%current-target-system when you call the procedure, then
another when you compute the derivation.
diff mbox series

Patch

diff --git a/gnu/packages/cross-base.scm b/gnu/packages/cross-base.scm
index 6ee7b315d8..ca3b41a4c7 100644
--- a/gnu/packages/cross-base.scm
+++ b/gnu/packages/cross-base.scm
@@ -306,6 +306,11 @@  (define* (cross-gcc target
 XGCC as the base compiler.  Use XBINUTILS as the associated cross-Binutils.
 If LIBC is false, then build a GCC that does not target a libc; otherwise,
 target that libc."
+  (define ld-wrapper
+    (make-ld-wrapper (string-append "ld-wrapper-" target)
+                     #:target (const target)
+                     #:binutils xbinutils))
+
   (package
     (inherit xgcc)
     (name (string-append "gcc-cross-"
@@ -313,8 +318,7 @@  (define* (cross-gcc target
                          target))
     (source
      (origin
-       (inherit
-        (package-source xgcc))
+       (inherit (package-source xgcc))
        (patches
         (append
          (origin-patches (package-source xgcc))
@@ -353,10 +357,7 @@  (define* (cross-gcc target
        ,@(cross-gcc-arguments target xgcc libc)))
 
     (native-inputs
-     `(("ld-wrapper-cross" ,(make-ld-wrapper
-                             (string-append "ld-wrapper-" target)
-                             #:target (const target)
-                             #:binutils xbinutils))
+     `(("ld-wrapper-cross" ,ld-wrapper)
        ("binutils-cross" ,xbinutils)
 
        ,@(let ((inputs (append (package-inputs xgcc)