diff mbox series

[bug#74253] transformations: Add multituned-package.

Message ID d3571383007eb50c7156d1ef88f9fb159b3fbb3d.1731062591.git.efraim@flashner.co.il
State New
Headers show
Series [bug#74253] transformations: Add multituned-package. | expand

Commit Message

Efraim Flashner Nov. 8, 2024, 10:44 a.m. UTC 
* guix/transformations.scm (package-tuned-for-psabi,
multituned-package): New variables.

Change-Id: I09ac7ae9fc2bcd9aa712b3c30fef807bc7d55895
---

This allows wrapping a package definition in multituned-package, ie:

(define-public opus
 (multituned-package
  (package
   ...)))

I'm not sure where to go with this patch from here. This will provide
the psabi libraries for x86_64 and powerpc64le so they get most of the
benefits from tuning for the architecture but without needing to specify
which architecture to tune for.  It should also provide a nice boost for
guix packs and docker images and the like.

The downside with using this by default is the larger package size due
to the extra versions of the libraries, and if it is used then the
regular --tune is disabled for that package.

I think adding it as a '--tune=generic' or '--tune=psabi' would be a
nice way to use it.

 guix/transformations.scm | 75 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)


base-commit: 2a6d96425eea57dc6dd48a2bec16743046e32e06

Comments

Ludovic Courtès Nov. 12, 2024, 10:31 a.m. UTC | #1 
Hi,

Efraim Flashner <efraim@flashner.co.il> skribis:

> * guix/transformations.scm (package-tuned-for-psabi,
> multituned-package): New variables.
>
> Change-Id: I09ac7ae9fc2bcd9aa712b3c30fef807bc7d55895
> ---
>
> This allows wrapping a package definition in multituned-package, ie:
>
> (define-public opus
>  (multituned-package
>   (package
>    ...)))
>
> I'm not sure where to go with this patch from here. This will provide
> the psabi libraries for x86_64 and powerpc64le so they get most of the
> benefits from tuning for the architecture but without needing to specify
> which architecture to tune for.  It should also provide a nice boost for
> guix packs and docker images and the like.
>
> The downside with using this by default is the larger package size due
> to the extra versions of the libraries, and if it is used then the
> regular --tune is disabled for that package.
>
> I think adding it as a '--tune=generic' or '--tune=psabi' would be a
> nice way to use it.

Should that be a package transformation though?  Could we instead have a
build system trick or the ‘multituned-package’ procedure exposed so
build the package several times and fill in lib/glibc-hwcaps?

That way, packagers would explicitly choose this technique for select
packages, which would then no longer need the ‘tunable?’ property.

The question becomes: how would we choose which packages is eligible to
this technique as opposed to ‘--tune’?  Intuitively, I would use that
for general-purpose packages like ‘opus’, but keep ‘--tune’ for more
niche/scientific packages.

WDYT?

Thanks,
Ludo’.
diff mbox series

Patch

diff --git a/guix/transformations.scm b/guix/transformations.scm
index ea8b7a08443..4787e016b5d 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -60,6 +60,7 @@  (define-module (guix transformations)
 
             tunable-package?
             tuned-package
+            multituned-package
 
             show-transformation-options-help
             transformation-option-key?
@@ -634,6 +635,80 @@  (define (tuned-package p micro-architecture)
        ;; call 'tuned-package' again on this one.
        ,@(alist-delete 'tunable? (package-properties p))))))
 
+(define (package-tuned-for-psabi p psabi)
+  (let ((base (tuned-package p psabi)))
+    (package/inherit base
+      (name (string-append (package-name base) "-" psabi))
+      (arguments
+       (substitute-keyword-arguments (package-arguments base)
+         ((#:configure-flags flags #~'())
+          #~(append
+              (list
+                #$@(if (eq? (build-system-name (package-build-system p)) ; not base
+                            (quote cmake-build-system))
+                       #~((string-append "-DCMAKE_INSTALL_LIBDIR=lib/glibc-hwcaps/"
+                                         #$psabi))
+                       #~((string-append "--libdir=" #$output
+                                         "/lib/glibc-hwcaps/" #$psabi))))
+              #$flags))
+         ((#:phases phases #~%standard-phases)
+          #~(modify-phases #$phases
+              (add-after 'install 'remove-extra-files
+                (lambda _
+                  (for-each (lambda (dir)
+                              (when (file-exists? (string-append #$output dir))
+                                (delete-file-recursively
+                                  (string-append #$output dir))))
+                            (list (string-append "/lib/glibc-hwcaps/"
+                                                 #$psabi "/cmake")
+                                  (string-append "/lib/glibc-hwcaps/"
+                                                 #$psabi "/pkgconfig")
+                                  "/bin" "/etc" "/include" "/libexec"
+                                  "/sbin" "/share" "/var")))))))))))
+
+(define (multituned-package p)
+  (package/inherit p
+    (arguments
+     (substitute-keyword-arguments (package-arguments p)
+       ((#:phases phases #~%standard-phases)
+        (if (or (target-x86-64?)
+                (target-ppc64le?))
+          #~(modify-phases #$phases
+              (add-after 'install 'install-optimized-libraries
+                (lambda* (#:key inputs outputs #:allow-other-keys)
+                  (let ((hwcaps "/lib/glibc-hwcaps/"))
+                    (for-each
+                      (lambda (psabi)
+                        (copy-recursively
+                          (string-append
+                            (assoc-ref inputs (string-append
+                                                #$(package-name p) "-" psabi))
+                            hwcaps psabi)
+                          (string-append #$output hwcaps psabi)))
+                      #$(cond ((target-x86-64?)
+                               #~(list "x86-64-v2" "x86-64-v3" "x86-64-v4"))
+                              ((target-ppc64le?)
+                               #~(list "power9" "power10"))
+                              (#t #~'())))))))
+          phases))))
+    (inputs
+     (cond ((target-x86-64?)
+            (modify-inputs (package-inputs p)
+              (append (package-tuned-for-psabi p "x86-64-v2")
+                      (package-tuned-for-psabi p "x86-64-v3")
+                      (package-tuned-for-psabi p "x86-64-v4"))))
+           ((target-ppc64le?)
+            (modify-inputs (package-inputs p)
+              (append (package-tuned-for-psabi p "power9")
+                      (package-tuned-for-psabi p "power10"))))
+           (#t (package-inputs p))))
+    ;; With the addition of the psABIs this package should not be tuned.
+    (properties
+     (if (or (target-x86-64?)
+             (target-ppc64le?))
+         '((alist-delete 'tunable? (package-properties p)))
+         (package-properties p)))))
+
 (define (tunable-package? package)
   "Return true if package PACKAGE is \"tunable\"--i.e., if tuning it for the
 host CPU is worthwhile."