[PATCH 0/2] Use guile-final for grafting.

Open

Details

2 participants

Efraim Flashner
Ludovic Courtès

Owner: unassigned

Submitted by: Efraim Flashner

Severity: normal

Debbugs page

Efraim Flashner wrote on 9 Apr 03:03 -0700

Recipients:(address . guix-patches@gnu.org)(name . Efraim Flashner)(address . efraim@flashner.co.il)

Message-ID:cover.1712656502.git.efraim@flashner.co.il

Over the years the grafting code has changed bit by bit, with various

attempts to speed it up. By switching the grafts to not use parallelism

in rewriting the leaves we can finally switch the guile-for-grafts to be

guile-final. The segfault is still there if we perform the grafts in

parallel, but I believe it is fast enough that it should be fine to do

them sequentially.

There's probably room in replace-store-references in (guix build graft)

for changes if we're not going to use guile-2.0 here anymore.

There are also a number of default guiles in (guix grafts) which should

maybe be %guile-for-grafts instead of %guile-for-build.

Efraim Flashner (2):

graft: Remove work-around for old guile.

graft: Perform grafts with guile-final.

guix/build/graft.scm | 55 +++++---------------------------------------

guix/packages.scm | 6 ++---

2 files changed, 8 insertions(+), 53 deletions(-)

base-commit: 51de844a0ff6ea224367a384092896bce6848b9f

prerequisite-patch-id: ea387a4f9d860397a26c840c11c8742f0ac70fc3

Efraim Flashner <efraim@flashner.co.il> רנשלפ םירפא

GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351

Confidentiality cannot be guaranteed on emails sent or received unencrypted

Efraim Flashner wrote on 9 Apr 03:06 -0700

[PATCH 1/2] graft: Remove work-around for old guile.

Recipients:(address . 70303@debbugs.gnu.org)(name . Efraim Flashner)(address . efraim@flashner.co.il)

Message-ID:3d8f8f3135f69cc88a7d0575ce533d489239ede8.1712656502.git.efraim@flashner.co.il

* guix/build/graft.scm (mkdir-p*): Remove function.

(rewrite-directory): Switch from mkdir-p* to mkdir-p.

Change-Id: Ib6a80648d271c19093c05af84acb967e069ccc19

---

guix/build/graft.scm | 31 ++-----------------------------

1 file changed, 2 insertions(+), 29 deletions(-)

Toggle diff (60 lines)diff --git a/guix/build/graft.scm b/guix/build/graft.scm
index 281dbaba6f..c8c7e33ab2 100644
--- a/guix/build/graft.scm
+++ b/guix/build/graft.scm
@@ -312,33 +312,6 @@ (define (exit-on-exception proc)
           (print-exception port #f key args)
           (primitive-exit 1))))))
 
-;; We need this as long as we support Guile < 2.0.13.
-(define* (mkdir-p* dir #:optional (mode #o755))
-  "This is a variant of 'mkdir-p' that works around
-<http://bugs.gnu.org/24659> by passing MODE explicitly in each 'mkdir' call."
-  (define absolute?
-    (string-prefix? "/" dir))
-
-  (define not-slash
-    (char-set-complement (char-set #\/)))
-
-  (let loop ((components (string-tokenize dir not-slash))
-             (root       (if absolute?
-                             ""
-                             ".")))
-    (match components
-      ((head tail ...)
-       (let ((path (string-append root "/" head)))
-         (catch 'system-error
-           (lambda ()
-             (mkdir path mode)
-             (loop tail path))
-           (lambda args
-             (if (= EEXIST (system-error-errno args))
-                 (loop tail path)
-                 (apply throw args))))))
-      (() #t))))
-
 (define* (rewrite-directory directory output mapping
                             #:optional (store (%store-directory)))
   "Copy DIRECTORY to OUTPUT, replacing strings according to MAPPING, a list of
@@ -387,7 +360,7 @@ (define* (rewrite-directory directory output mapping
   (define (rewrite-leaf file)
     (let ((stat (lstat file))
           (dest (destination file)))
-      (mkdir-p* (dirname dest))
+      (mkdir-p (dirname dest))
       (case (stat:type stat)
         ((symlink)
          (let ((target (readlink file)))
@@ -406,7 +379,7 @@ (define* (rewrite-directory directory output mapping
                                            store)
                  (chmod output (stat:perms stat)))))))
         ((directory)
-         (mkdir-p* dest))
+         (mkdir-p dest))
         (else
          (error "unsupported file type" stat)))))
 
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

Efraim Flashner wrote on 9 Apr 03:06 -0700

[PATCH 2/2] graft: Perform grafts with guile-final.

Recipients:(address . 70303@debbugs.gnu.org)(name . Efraim Flashner)(address . efraim@flashner.co.il)

Message-ID:bb20ba66301eaa5c5f0df59eddb42f7ad11f4a90.1712656502.git.efraim@flashner.co.il

* guix/build/graft.scm (rewrite-directory): Rewrite store directories in
individual files sequentially.
(exit-on-exception): Remove procedure.
* guix/packages.scm (guile-for-grafts): Switch to guile-final.

Change-Id: I50f7b23a3ceff8bb1495dc1f4bc772746147d924
---
 guix/build/graft.scm | 24 ++++--------------------
 guix/packages.scm    |  6 ++----
 2 files changed, 6 insertions(+), 24 deletions(-)

Toggle diff (63 lines)diff --git a/guix/build/graft.scm b/guix/build/graft.scm
index c8c7e33ab2..7fc5ecba99 100644
--- a/guix/build/graft.scm
+++ b/guix/build/graft.scm
@@ -299,19 +299,6 @@ (define (rename-matching-files directory mapping)
                                (string-append (dirname file) "/" target))))
               matches)))
 
-(define (exit-on-exception proc)
-  "Return a procedure that wraps PROC so that 'primitive-exit' is called when
-an exception is caught."
-  (lambda (arg)
-    (catch #t
-      (lambda ()
-        (proc arg))
-      (lambda (key . args)
-        ;; Since ports are not thread-safe as of Guile 2.0, reopen stderr.
-        (let ((port (fdopen 2 "w0")))
-          (print-exception port #f key args)
-          (primitive-exit 1))))))
-
 (define* (rewrite-directory directory output mapping
                             #:optional (store (%store-directory)))
   "Copy DIRECTORY to OUTPUT, replacing strings according to MAPPING, a list of
@@ -383,13 +370,10 @@ (define* (rewrite-directory directory output mapping
         (else
          (error "unsupported file type" stat)))))
 
-  ;; Use 'exit-on-exception' to force an exit upon I/O errors, given that
-  ;; 'n-par-for-each' silently swallows exceptions.
-  ;; See <http://bugs.gnu.org/23581>.
-  (n-par-for-each (parallel-job-count)
-                  (exit-on-exception rewrite-leaf)
-                  (find-files directory (const #t)
-                              #:directories? #t))
+  ;; n-par-for-each can lead to segfaults in the grafting code.
+  (for-each rewrite-leaf
+            (find-files directory (const #t)
+                        #:directories? #t))
   (rename-matching-files output mapping))
 
 (define %graft-hooks
diff --git a/guix/packages.scm b/guix/packages.scm
index 930b1a3b0e..80642eb049 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -882,10 +882,8 @@ (define (default-guile)
 
 (define (guile-for-grafts)
   "Return the Guile package used to build grafting derivations."
-  ;; Guile 2.2 would not work due to <https://bugs.gnu.org/28211> when
-  ;; grafting packages.
-  (let ((distro (resolve-interface '(gnu packages guile))))
-    (module-ref distro 'guile-2.0)))
+  (let ((distro (resolve-interface '(gnu packages commencement))))
+    (module-ref distro 'guile-final)))
 
 (define* (default-guile-derivation #:optional (system (%current-system)))
   "Return the derivation for SYSTEM of the default Guile package used to run
-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

Ludovic Courtès wrote on 15 Apr 13:28 -0700

Re: [bug#70303] [PATCH 0/2] Use guile-final for grafting.

Recipients:(name . Efraim Flashner)(address . efraim@flashner.co.il)

Message-ID:87cyqqz7z2.fsf@gnu.org

Howdy!

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

Toggle quote (7 lines)

> Over the years the grafting code has changed bit by bit, with various

> attempts to speed it up. By switching the grafts to not use parallelism

> in rewriting the leaves we can finally switch the guile-for-grafts to be

> guile-final. The segfault is still there if we perform the grafts in

> parallel, but I believe it is fast enough that it should be fine to do

> them sequentially.

Could you time the grafting derivation of, say, libreoffice or

ungoogled-chromium?

Typically I’d do it along these lines:

Toggle snippet (17 lines)$ guix build libreoffice
/gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1
$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
$ time guix build libreoffice --check
The following graft will be made:
   /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
applying 137 grafts for libreoffice-7.6.3.1 ...
grafting '/gnu/store/5flppg3h8y235di2ilr3sx878gfl82db-libreoffice-7.6.3.1' -> '/gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1'...
successfully built /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
successfully built /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
/gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1

real    0m14.921s
user    0m7.588s
sys     0m0.389s

(That’s on my 4-core i7.)

It’s a bummer that the segfault is still there. I remember week-long

‘rr’ debugging sessions in the past, where I did find a few issues; I

should try again but uh…

Toggle quote (3 lines)

> There's probably room in replace-store-references in (guix build graft)

> for changes if we're not going to use guile-2.0 here anymore.

I believe ‘tests/grafts.scm’ may run some of this code under Guile 2.0

(using the ‘guile-bootstrap’ tarball provided for the tests).

Thanks,

Ludo’.

Efraim Flashner wrote on 16 Apr 07:56 -0700

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:Zh6RhC4IHoIcFfbV@pbp

On Mon, Apr 15, 2024 at 10:28:17PM +0200, Ludovic Courtès wrote:

Toggle quote (36 lines)> Howdy!
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > Over the years the grafting code has changed bit by bit, with various
> > attempts to speed it up.  By switching the grafts to not use parallelism
> > in rewriting the leaves we can finally switch the guile-for-grafts to be
> > guile-final.  The segfault is still there if we perform the grafts in
> > parallel, but I believe it is fast enough that it should be fine to do
> > them sequentially.
> 
> Could you time the grafting derivation of, say, libreoffice or
> ungoogled-chromium?
> 
> Typically I’d do it along these lines:
> 
> --8<---------------cut here---------------start------------->8---
> $ guix build libreoffice
> /gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1
> $ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
> $ time guix build libreoffice --check
> The following graft will be made:
>    /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
> applying 137 grafts for libreoffice-7.6.3.1 ...
> grafting '/gnu/store/5flppg3h8y235di2ilr3sx878gfl82db-libreoffice-7.6.3.1' -> '/gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1'...
> successfully built /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
> successfully built /gnu/store/wdjqpxm2kbdvq7qlrzyjxb244zn3s3bv-libreoffice-7.6.3.1.drv
> /gnu/store/24is7ypdx6sm56mkclxdx4hyj7yg4smb-libreoffice-7.6.3.1
> 
> real    0m14.921s
> user    0m7.588s
> sys     0m0.389s
> --8<---------------cut here---------------end--------------->8---
> 
> (That’s on my 4-core i7.)

I'm currently vising my parents for a few weeks during the Passover
break, and I've already lost connection to my desktop at home, so I'm
currently running the tests on my pinebook pro.

The first one is with the patches applied, the second one is without the
patches. I have the guix-daemon on my pinebook pro to use 3 cores, but
I'm not sure how much that would be honored by the grafting code.

(ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
Password:
(ins)efraim@pbp ~/workspace/guix$ time ./pre-inst-env guix build libreoffice --check
The following graft will be made:
   /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
applying 137 grafts for libreoffice-7.6.3.1 ...
grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1'...
successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
/gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1

real    1m35.537s
user    0m32.328s
sys     0m2.521s
(ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
(ins)efraim@pbp ~/workspace/guix$ time guix build libreoffice --check
The following graft will be made:
   /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
applying 137 grafts for libreoffice-7.6.3.1 ...
grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1'...
successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
/gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1

real    1m20.573s
user    0m29.688s
sys     0m2.346s

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

Ludovic Courtès wrote on 4 May 08:03 -0700

Recipients:(name . Efraim Flashner)(address . efraim@flashner.co.il)

Message-ID:87ttjdty9m.fsf@gnu.org

Hi!

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

Toggle quote (32 lines)> The first one is with the patches applied, the second one is without the
> patches. I have the guix-daemon on my pinebook pro to use 3 cores, but
> I'm not sure how much that would be honored by the grafting code.
>
> (ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
> Password:
> (ins)efraim@pbp ~/workspace/guix$ time ./pre-inst-env guix build libreoffice --check
> The following graft will be made:
>    /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
> applying 137 grafts for libreoffice-7.6.3.1 ...
> grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1'...
> successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
> successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
> /gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1
>
> real    1m35.537s
> user    0m32.328s
> sys     0m2.521s
> (ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
> (ins)efraim@pbp ~/workspace/guix$ time guix build libreoffice --check
> The following graft will be made:
>    /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
> applying 137 grafts for libreoffice-7.6.3.1 ...
> grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1'...
> successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
> successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
> /gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1
>
> real    1m20.573s
> user    0m29.688s
> sys     0m2.346s

That’s an 18% slowdown. Could you make several runs to see how stable

that is?

Thanks,

Ludo’.

Ludovic Courtès wrote on 18 Jul 08:14 -0700

Recipients:(name . Efraim Flashner)(address . efraim@flashner.co.il)

Message-ID:87sew6vijw.fsf@gnu.org

Hi Efraim,

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (37 lines)> Efraim Flashner <efraim@flashner.co.il> skribis:
>
>> The first one is with the patches applied, the second one is without the
>> patches. I have the guix-daemon on my pinebook pro to use 3 cores, but
>> I'm not sure how much that would be honored by the grafting code.
>>
>> (ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
>> Password:
>> (ins)efraim@pbp ~/workspace/guix$ time ./pre-inst-env guix build libreoffice --check
>> The following graft will be made:
>>    /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
>> applying 137 grafts for libreoffice-7.6.3.1 ...
>> grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1'...
>> successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
>> successfully built /gnu/store/i2aml4p5yg7h090bdzjpaqmds47g26d8-libreoffice-7.6.3.1.drv
>> /gnu/store/1cfjx934czp641v4fmwsz2js7158ivgm-libreoffice-7.6.3.1
>>
>> real    1m35.537s
>> user    0m32.328s
>> sys     0m2.521s
>> (ins)efraim@pbp ~/workspace/guix$ sudo sh -c 'echo 3 > /proc/sys/vm/drop_caches'
>> (ins)efraim@pbp ~/workspace/guix$ time guix build libreoffice --check
>> The following graft will be made:
>>    /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
>> applying 137 grafts for libreoffice-7.6.3.1 ...
>> grafting '/gnu/store/yd9slkfhdl8lzhhmhm40airimwb6yhj5-libreoffice-7.6.3.1' -> '/gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1'...
>> successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
>> successfully built /gnu/store/2f6i7r77z8msbjlspsp0aq5vlpjqnifp-libreoffice-7.6.3.1.drv
>> /gnu/store/xm3q8qsns8qqybq47zvv70n0y0qs4r65-libreoffice-7.6.3.1
>>
>> real    1m20.573s
>> user    0m29.688s
>> sys     0m2.346s
>
> That’s an 18% slowdown.  Could you make several runs to see how stable
> that is?

Looks like this patch fell through the cracks.

Maybe we should just go ahead and apply it. Depending on Guile 2.0 for

this isn’t viable in the long term anyway.

WDYT?

Ludo’.

Efraim Flashner wrote on 22 Jul 14:54 -0700

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:Zp7VJ35eFGQKF6Bu@pbp

Attachment: file

Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 70303@patchwise.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 70303

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date