Re: [PATCH] build: pull: Compile .scm files in one process.

[taylanbayirli@gmail.com (Taylan Ulrich Bayırlı/Kammer)]

[-- Attachment #1: Type: text/plain, Size: 2616 bytes --]

ludo@gnu.org (Ludovic Courtès) writes:

> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:
>
>> From 0eaf7d4c264dc531718a4c0b933323f48ea91930 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Taylan=20Ulrich=20Bay=C4=B1rl=C4=B1/Kammer?=
>>  <taylanbayirli@gmail.com>
>> Date: Thu, 5 Nov 2015 23:43:20 +0100
>> Subject: [PATCH] build: pull: Compile .scm files in one process.
>>
>> * guix/build/pull.scm (call-with-process): Removed procedure.
>> (report-build-progress): Removed procedure.
>> (p-for-each): Removed procedure.
>
> Rather:
>
> … (call-with-process, report-build-progress, p-for-each): Remove.

Done.

>> +        (let ((mutex (make-mutex))
>> +              (completed 0))
>> +          (par-for-each
>> +           (lambda (file)
>> +             (with-mutex mutex
>> +               (display #\cr log-port)
>> +               (format log-port "compiling...\t~5,1f% of ~d files" ;FIXME: i18n
>> +                       (* 100. (/ completed total)) total)
>> +               (force-output log-port)
>> +               (format debug-port "~%compiling '~a'...~%" file))
>> +             (let ((go (string-append (string-drop-right file 4) ".go")))
>> +               (compile-file file
>> +                             #:output-file go
>> +                             #:opts %auto-compilation-options))
>> +             (with-mutex mutex
>> +               (set! completed (+ 1 completed))))
>> +           files)))))
>
> Does it actually work reliably?  :-)
>
> I think we’re in trouble if DEBUG-PORT is a real port because ports
> aren’t thread-safe in 2.0, and ‘compile-file’ might write to it.  Void
> ports seem to be OK because their ‘write’ method doesn’t do anything.
>
> So I think we have to do things sequentially when DEBUG-PORT is
> non-void.
>
> I’m also concerned about modules accessed by concurrent calls to
> ‘compile-file’ because modules and hash tables aren’t thread-safe
> either.
>
> WDYT?
>
> Thanks to the two of you!  Sounds like we’ll soon have a less slow or
> even a fast ‘guix pull’.  ;-)

Hmm, how about this one?  I guess it could still garble output if
'compile-file' writes things in small pieces that belong together, but
at least it's sure not to crash.  Compile-file doesn't seem to write a
lot either; one probably won't see much garbled output in practice.
Maybe the speed gain makes this acceptable.

Re. modules, AFAIUI compile-file shouldn't do any further mutations on
modules, since we load them in advance.  Maybe Andy can tell how safe
this is.


[-- Attachment #2: 0001-build-pull-Compile-.scm-files-in-one-process.patch --]
[-- Type: text/x-diff, Size: 8348 bytes --]

From 78be6d09d2d4c0a563be14c66ac2a1a345ff9b1d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Taylan=20Ulrich=20Bay=C4=B1rl=C4=B1/Kammer?=
 <taylanbayirli@gmail.com>
Date: Thu, 5 Nov 2015 23:43:20 +0100
Subject: [PATCH] build: pull: Compile .scm files in one process.

* guix/build/pull.scm (call-with-process, report-build-progress)
(p-for-each): Remove.
(thread-safe-port): New procedure.
(build-guix): Load and compile files in one process.
---
 guix/build/pull.scm | 147 +++++++++++++++++++---------------------------------
 1 file changed, 54 insertions(+), 93 deletions(-)

diff --git a/guix/build/pull.scm b/guix/build/pull.scm
index 281be23..75bce7c 100644
--- a/guix/build/pull.scm
+++ b/guix/build/pull.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2015 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -22,6 +23,7 @@
   #:use-module (ice-9 ftw)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
+  #:use-module (ice-9 threads)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
@@ -33,75 +35,18 @@
 ;;;
 ;;; Code:
 
-(define (call-with-process thunk)
-  "Run THUNK in a separate process that will return 0 if THUNK terminates
-normally, and 1 if an exception is raised."
-  (match (primitive-fork)
-    (0
-     (catch #t
-       (lambda ()
-         (thunk)
-         (primitive-exit 0))
-       (lambda (key . args)
-         (print-exception (current-error-port) #f key args)
-         (primitive-exit 1))))
-    (pid
-     #t)))
-
-(define* (report-build-progress total completed cont
-                                #:optional (log-port (current-error-port)))
-  "Report that COMPLETED out of TOTAL files have been completed, and call
-CONT."
-  (display #\cr log-port)
-  (format log-port "compiling...\t~5,1f% of ~d files" ;FIXME: i18n
-          (* 100. (/ completed total)) total)
-  (force-output log-port)
-  (cont))
-
-(define* (p-for-each proc lst
-                     #:optional (max-processes (current-processor-count))
-                     #:key (progress report-build-progress))
-  "Invoke PROC for each element of LST in a separate process, using up to
-MAX-PROCESSES processes in parallel.  Call PROGRESS at each step, passing it
-the continuation.  Raise an error if one of the processes exit with non-zero."
-  (define total
-    (length lst))
-
-  (define (wait-for-one-process)
-    (match (waitpid WAIT_ANY)
-      ((_ . status)
-       (unless (zero? (status:exit-val status))
-         (error "process failed" proc status)))))
-
-  (let loop ((lst       lst)
-             (running   0)
-             (completed 0))
-    (match lst
-      (()
-       (or (zero? running)
-           (let ((running   (- running 1))
-                 (completed (+ completed 1)))
-             (wait-for-one-process)
-             (progress total completed
-                       (lambda ()
-                         (loop lst running completed))))))
-      ((head . tail)
-       (if (< running max-processes)
-           (let ((running (+ 1 running)))
-             (call-with-process (cut proc head))
-             (progress total completed
-                       (lambda ()
-                         (loop tail running completed))))
-           (let ((running   (- running 1))
-                 (completed (+ completed 1)))
-             (wait-for-one-process)
-             (progress total completed
-                       (lambda ()
-                         (loop lst running completed)))))))))
+(define (thread-safe-port port)
+  (define mutex (make-mutex))
+  (define (wrap proc)
+    (lambda args
+      (with-mutex mutex (apply proc args))))
+  (make-soft-port
+   (vector (wrap write-char) (wrap display) (wrap force-output) #f #f) "w"))
 
 (define* (build-guix out source
                      #:key gcrypt
-                     (debug-port (%make-void-port "w")))
+                     (debug-port (%make-void-port "w"))
+                     (log-port (current-error-port)))
   "Build and install Guix in directory OUT using SOURCE, a directory
 containing the source code.  Write any debugging output to DEBUG-PORT."
   (setvbuf (current-output-port) _IOLBF)
@@ -130,33 +75,49 @@ containing the source code.  Write any debugging output to DEBUG-PORT."
     (set! %load-path (cons out %load-path))
     (set! %load-compiled-path (cons out %load-compiled-path))
 
-    ;; Compile the .scm files.  Do that in independent processes, à la
-    ;; 'make -j', to work around <http://bugs.gnu.org/15602> (FIXME).
-    ;; This ensures correctness, but is overly conservative and slow.
-    ;; The solution initially implemented (and described in the bug
-    ;; above) was slightly faster but consumed memory proportional to the
-    ;; number of modules, which quickly became unacceptable.
-    (p-for-each (lambda (file)
-                  (let ((go (string-append (string-drop-right file 4)
-                                           ".go")))
-                    (format debug-port "~%compiling '~a'...~%" file)
-                    (parameterize ((current-warning-port debug-port))
-                      (compile-file file
-                                    #:output-file go
-                                    #:opts
-                                    %auto-compilation-options))))
-
-                (filter (cut string-suffix? ".scm" <>)
-
-                        ;; Build guix/*.scm before gnu/*.scm to speed
-                        ;; things up.
-                        (sort (find-files out "\\.scm")
-                              (let ((guix (string-append out "/guix"))
-                                    (gnu  (string-append out "/gnu")))
-                                (lambda (a b)
-                                  (or (and (string-prefix? guix a)
-                                           (string-prefix? gnu b))
-                                      (string<? a b))))))))
+    ;; Compile the .scm files.  Load all the files before compiling them to
+    ;; work around <http://bugs.gnu.org/15602> (FIXME).
+    (let* ((files
+            ;; gnu.scm depends on many other modules, so to avoid an early
+            ;; progress report stall, we move it to the end.
+            (sort (filter (cut string-suffix? ".scm" <>)
+                          (find-files out "\\.scm"))
+                  (lambda (a b)
+                    (or (string-prefix? (string-append out "/gnu.scm") b)
+                        (string<? a b)))))
+           (total (length files)))
+      (let loop ((files files)
+                 (completed 0))
+        (match files
+          (() *unspecified*)
+          ((file . files)
+           (display #\cr log-port)
+           (format log-port "loading...\t~5,1f% of ~d files" ;FIXME: i18n
+                   (* 100. (/ completed total)) total)
+           (force-output log-port)
+           (parameterize ((current-warning-port debug-port))
+             (save-module-excursion
+              (lambda () (primitive-load file))))
+           (loop files (+ 1 completed)))))
+      (newline)
+      (let ((mutex (make-mutex))
+            (completed 0))
+        (parameterize ((current-warning-port (thread-safe-port debug-port)))
+          (par-for-each
+           (lambda (file)
+             (with-mutex mutex
+               (display #\cr log-port)
+               (format log-port "compiling...\t~5,1f% of ~d files" ;FIXME: i18n
+                       (* 100. (/ completed total)) total)
+               (force-output log-port)
+               (format debug-port "~%compiling '~a'...~%" file))
+             (let ((go (string-append (string-drop-right file 4) ".go")))
+               (compile-file file
+                             #:output-file go
+                             #:opts %auto-compilation-options))
+             (with-mutex mutex
+               (set! completed (+ 1 completed))))
+           files)))))
 
   ;; Remove the "fake" (guix config).
   (delete-file (string-append out "/guix/config.scm"))
-- 
2.5.0