[PATCH] Makefile: Speed up .go compilation.

[PATCH] Makefile: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

This does the same thing as the 'guix pull' patch, but for our Makefile.

Improvement suggestions welcome, since it's pretty hacky.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-build-Speed-up-.go-compilation.patch --]
[-- Type: text/x-diff, Size: 3936 bytes --]

From f205496e8f08c3621d2ffe2c802da1f9e367e2b9 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:42:45 +0100
Subject: [PATCH 1/2] build: Speed up .go compilation.

* build-aux/compile-all.scm: New file.
* Makefile.am: Call build-aux/compile-all.scm to compile many .scm files
  in a single process.
---
 Makefile.am               | 22 ++++++++--------------
 build-aux/compile-all.scm | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 14 deletions(-)
 create mode 100644 build-aux/compile-all.scm

diff --git a/Makefile.am b/Makefile.am
index 67d483b..bf73823 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -335,14 +335,6 @@ CLEANFILES =					\
   $(GOBJECTS)					\
   $(SCM_TESTS:tests/%.scm=%.log)
 
-AM_V_GUILEC = $(AM_V_GUILEC_$(V))
-AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
-AM_V_GUILEC_0 = @echo "  GUILEC" $@;
-
-# Flags passed to 'guild compile'.
-GUILD_COMPILE_FLAGS =				\
-  -Wformat -Wunbound-variable -Warity-mismatch
-
 # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
 # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
 # there that are newer than the local .scm files (for instance because the
@@ -352,14 +344,16 @@ GUILD_COMPILE_FLAGS =				\
 #
 # XXX: Use the C locale for when Guile lacks
 # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
-.scm.go:
-	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
+%.go: make-go ; @:
+make-go: $(MODULES)
+	for f in $^; do							\
+          $(MKDIR_P) `dirname "$$f"` ;					\
+	done ;								\
 	unset GUILE_LOAD_COMPILED_PATH ;				\
 	LC_ALL=C							\
 	$(top_builddir)/pre-inst-env					\
-	$(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)"	\
-	  $(GUILD_COMPILE_FLAGS) --target="$(host)"			\
-	  -o "$@" "$<"
+	$(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)"		\
+	  --no-auto-compile -s build-aux/compile-all.scm $(host) $^
 
 SUFFIXES = .go
 
@@ -451,6 +445,6 @@ assert-final-inputs-self-contained:
 	$(top_builddir)/pre-inst-env "$(GUILE)"				\
 	  "$(top_srcdir)/build-aux/check-final-inputs-self-contained.scm"
 
-.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go
+.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go make-go
 .PHONY: assert-no-store-file-names assert-binaries-available
 .PHONY: assert-final-inputs-self-contained
diff --git a/build-aux/compile-all.scm b/build-aux/compile-all.scm
new file mode 100644
index 0000000..b97ce8f
--- /dev/null
+++ b/build-aux/compile-all.scm
@@ -0,0 +1,41 @@
+(use-modules (system base target)
+             (ice-9 threads))
+
+(define compile-options '(format unbound-variable arity-mismatch))
+
+(define (file-mtime<? f1 f2)
+  (< (stat:mtime (stat f1))
+     (stat:mtime (stat f2))))
+
+(define (scm->go file)
+  (string-append (string-drop-right file 4) ".go"))
+
+(let* ((args (cdr (command-line)))
+       (target (car args))
+       (files (cdr args)))
+  (for-each
+   (lambda (file)
+     (let ((go (scm->go file)))
+       (unless (and (file-exists? go)
+                    (file-mtime<? file go))
+         (format #t "  LOAD ~s~%" file)
+         (save-module-excursion
+          (lambda ()
+            (primitive-load file))))))
+   files)
+  (with-target target
+    (lambda ()
+      (let ((mutex (make-mutex)))
+        (par-for-each
+         (lambda (file)
+           (let ((go (scm->go file)))
+             (unless (and (file-exists? go)
+                          (file-mtime<? file go))
+               (with-mutex mutex
+                 (format #t "  GUILEC ~s~%" file)
+                 (force-output))
+               (compile-file file #:output-file go #:opts compile-options)
+               (with-mutex mutex
+                 (format #t "  WROTE ~s~%" go)
+                 (force-output)))))
+         files)))))
-- 
2.5.0

[PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

Here's an updated version that uses the same strategy as the one we
settled on for 'guix pull'.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-build-Speed-up-.go-compilation.patch --]
[-- Type: text/x-diff, Size: 4043 bytes --]

From 220a8caed6da22e349545899d5c51083bb3a8ac5 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:42:45 +0100
Subject: [PATCH] build: Speed up .go compilation.

* build-aux/compile-all.scm: New file.
* Makefile.am: Call build-aux/compile-all.scm to compile many .scm files
  in a single process.
---
 Makefile.am               | 22 ++++++++--------------
 build-aux/compile-all.scm | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 14 deletions(-)
 create mode 100644 build-aux/compile-all.scm

diff --git a/Makefile.am b/Makefile.am
index 760caed..4aa459d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -341,14 +341,6 @@ CLEANFILES =					\
   $(GOBJECTS)					\
   $(SCM_TESTS:tests/%.scm=%.log)
 
-AM_V_GUILEC = $(AM_V_GUILEC_$(V))
-AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
-AM_V_GUILEC_0 = @echo "  GUILEC" $@;
-
-# Flags passed to 'guild compile'.
-GUILD_COMPILE_FLAGS =				\
-  -Wformat -Wunbound-variable -Warity-mismatch
-
 # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
 # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
 # there that are newer than the local .scm files (for instance because the
@@ -358,14 +350,16 @@ GUILD_COMPILE_FLAGS =				\
 #
 # XXX: Use the C locale for when Guile lacks
 # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
-.scm.go:
-	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
+%.go: make-go ; @:
+make-go: $(MODULES)
+	for f in $^; do							\
+          $(MKDIR_P) `dirname "$$f"` ;					\
+	done ;								\
 	unset GUILE_LOAD_COMPILED_PATH ;				\
 	LC_ALL=C							\
 	$(top_builddir)/pre-inst-env					\
-	$(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)"	\
-	  $(GUILD_COMPILE_FLAGS) --target="$(host)"			\
-	  -o "$@" "$<"
+	$(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)"		\
+	  --no-auto-compile -s build-aux/compile-all.scm $(host) $^
 
 SUFFIXES = .go
 
@@ -457,6 +451,6 @@ assert-final-inputs-self-contained:
 	$(top_builddir)/pre-inst-env "$(GUILE)"				\
 	  "$(top_srcdir)/build-aux/check-final-inputs-self-contained.scm"
 
-.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go
+.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go make-go
 .PHONY: assert-no-store-file-names assert-binaries-available
 .PHONY: assert-final-inputs-self-contained
diff --git a/build-aux/compile-all.scm b/build-aux/compile-all.scm
new file mode 100644
index 0000000..f546822
--- /dev/null
+++ b/build-aux/compile-all.scm
@@ -0,0 +1,44 @@
+(use-modules (system base target)
+             (ice-9 threads))
+
+(define compile-options '(format unbound-variable arity-mismatch))
+
+(define (file-mtime<? f1 f2)
+  (< (stat:mtime (stat f1))
+     (stat:mtime (stat f2))))
+
+(define (scm->go file)
+  (string-append (string-drop-right file 4) ".go"))
+
+(define (file->module file)
+  (map string->symbol
+       (string-split (string-drop-right file 4) #\/)))
+
+(let* ((args (cdr (command-line)))
+       (target (car args))
+       (files (cdr args)))
+  (for-each
+   (lambda (file)
+     (let ((go (scm->go file)))
+       (unless (and (file-exists? go)
+                    (file-mtime<? file go))
+         (let ((module (file->module file)))
+           (format #t "  LOAD ~s~%" module)
+           (resolve-interface module)))))
+   files)
+  (with-target target
+    (lambda ()
+      (let ((mutex (make-mutex)))
+        (par-for-each
+         (lambda (file)
+           (let ((go (scm->go file)))
+             (unless (and (file-exists? go)
+                          (file-mtime<? file go))
+               (with-mutex mutex
+                 (format #t "  GUILEC ~s~%" file)
+                 (force-output))
+               (compile-file file #:output-file go #:opts compile-options)
+               (with-mutex mutex
+                 (format #t "  WROTE ~s~%" go)
+                 (force-output)))))
+         files)))))
-- 
2.6.3

Re: [PATCH] build: Speed up .go compilation.

[Ludovic Courtès]

taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:

> Here's an updated version that uses the same strategy as the one we
> settled on for 'guix pull'.

So, what speedup to you get compared to ‘make -jN’?

> From 220a8caed6da22e349545899d5c51083bb3a8ac5 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:42:45 +0100
> Subject: [PATCH] build: Speed up .go compilation.
>
> * build-aux/compile-all.scm: New file.
> * Makefile.am: Call build-aux/compile-all.scm to compile many .scm files
>   in a single process.

Rather:

  * Makefile.am (%.go, make-go): New rules.

Also, the new script must be added to ‘EXTRA_DIST’.

> +++ b/build-aux/compile-all.scm

Please add a copyright/license header.

> +(define (file->module file)
> +  (map string->symbol
> +       (string-split (string-drop-right file 4) #\/)))

Does this work with out-of-tree builds?

> +(let* ((args (cdr (command-line)))
> +       (target (car args))
> +       (files (cdr args)))

Please use ‘match’ for the win!  :-)

> +  (for-each
> +   (lambda (file)
> +     (let ((go (scm->go file)))
> +       (unless (and (file-exists? go)
> +                    (file-mtime<? file go))
> +         (let ((module (file->module file)))
> +           (format #t "  LOAD ~s~%" module)
> +           (resolve-interface module)))))
> +   files)

Make the ‘lambda’ a named top-level procedure, for clarity.  Also add a
reference to the evil bug that makes this hack necessary in the first
place.

Maybe it would be clearer to have:

  (define (file-needs-compilation? file)
    (file-mtime<? (scm->go file) file))

and then to:

  (let ((files (filter file-needs-compilation? files)))
    ;; …
    )

> +  (with-target target
> +    (lambda ()
> +      (let ((mutex (make-mutex)))
> +        (par-for-each
> +         (lambda (file)
> +           (let ((go (scm->go file)))
> +             (unless (and (file-exists? go)
> +                          (file-mtime<? file go))
> +               (with-mutex mutex
> +                 (format #t "  GUILEC ~s~%" file)
> +                 (force-output))
> +               (compile-file file #:output-file go #:opts compile-options)
> +               (with-mutex mutex
> +                 (format #t "  WROTE ~s~%" go)
> +                 (force-output)))))
> +         files)))))

Ditto: name the lambda.

I would use ~a instead of ~s, to match the current output, and remove
the “WROTE” output.

Could you send an updated patch?

It would be awesome if you could check that ‘make distcheck’ still
passes, and also make sure things behave correctly when modifying just
one file and running ‘make’, things like that.

Thank you!

Ludo’.

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

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

> So, what speedup to you get compared to ‘make -jN’?

It should have the same improvement as 'guix pull' since it does
basically the same thing.  I measured to make things concrete, and on my
machine the run time of 'make -j4' decreases from ~26m to ~6m.

>> * Makefile.am: Call build-aux/compile-all.scm to compile many .scm files
>>   in a single process.
>
> Rather:
>
>   * Makefile.am (%.go, make-go): New rules.
>
> Also, the new script must be added to ‘EXTRA_DIST’.

Done and done.  Also added a part to the commit message for EXTRA_DIST.

>> +++ b/build-aux/compile-all.scm
>
> Please add a copyright/license header.

Whoops, done.

>> +(define (file->module file)
>> +  (map string->symbol
>> +       (string-split (string-drop-right file 4) #\/)))
>
> Does this work with out-of-tree builds?

Good catch.  There were a few reasons it failed, all fixed now.

(There were also existing problems with out-of-tree builds; see other
patch I sent.)

>> +(let* ((args (cdr (command-line)))
>> +       (target (car args))
>> +       (files (cdr args)))
>
> Please use ‘match’ for the win!  :-)

Done.  I really need to burn that into my coding habits.

>> +  (for-each
>> +   (lambda (file)
>> +     (let ((go (scm->go file)))
>> +       (unless (and (file-exists? go)
>> +                    (file-mtime<? file go))
>> +         (let ((module (file->module file)))
>> +           (format #t "  LOAD ~s~%" module)
>> +           (resolve-interface module)))))
>> +   files)
>
> Make the ‘lambda’ a named top-level procedure, for clarity.  Also add a
> reference to the evil bug that makes this hack necessary in the first
> place.
>
> Maybe it would be clearer to have:
>
>   (define (file-needs-compilation? file)
>     (file-mtime<? (scm->go file) file))
>
> and then to:
>
>   (let ((files (filter file-needs-compilation? files)))
>     ;; …
>     )

Indeed, done.

>> +  (with-target target
>> +    (lambda ()
>> +      (let ((mutex (make-mutex)))
>> +        (par-for-each
>> +         (lambda (file)
>> +           (let ((go (scm->go file)))
>> +             (unless (and (file-exists? go)
>> +                          (file-mtime<? file go))
>> +               (with-mutex mutex
>> +                 (format #t "  GUILEC ~s~%" file)
>> +                 (force-output))
>> +               (compile-file file #:output-file go #:opts compile-options)
>> +               (with-mutex mutex
>> +                 (format #t "  WROTE ~s~%" go)
>> +                 (force-output)))))
>> +         files)))))
>
> Ditto: name the lambda.

Here the lambda cannot entirely be factored out since the 'mutex' from
the lexical scope is needed.  I still factored out the actual code and
pass the mutex as an argument to the procedure in a minimal lambda, as
seen in the new patch below; I hope it's readable this way.

> I would use ~a instead of ~s, to match the current output, and remove
> the “WROTE” output.

Done.

> Could you send an updated patch?

Attached at the bottom.

> It would be awesome if you could check that ‘make distcheck’ still
> passes, and also make sure things behave correctly when modifying just
> one file and running ‘make’, things like that.

I'm having headaches with distcheck.  Currently it bails out because I'm
missing tex.  Debian's version is apparently too old, and Guix's version
is huge and has been downloading for many hours now.  I'll report back
again when I'm done with that.

I've ran 'make' on my branch containing this patch a couple times in the
past after rebasing on master, so that a subset of the .scm files would
be recompiled, and didn't have issues.  I can't imagine a way it would
break either, for any given subset of all the .scm files.  I hope I'm
not overseeing anything.

> Thank you!

Thanks for the review! :-)

Other notable changes in this version of the patch:

- I noticed guix/config.scm and guix/tests.scm are not in MODULES in
  Makefile.am (intentionally?), so I added them to the make-go rule.

- I removed the MKDIR_P loop in the make-go rule, and do the equivalent
  in the Scheme code now.

- The target host and top source directory are now passed to the script
  via lowercase environment variables, which makes the code a little
  simpler.  I hope this is stylistically fine.

> Ludo’.

Taylan



[-- Attachment #2: 0001-build-Speed-up-.go-compilation.patch --]
[-- Type: text/x-diff, Size: 5921 bytes --]

From 697950b82ea86f7b7438e586bbf4efae3e87d8f8 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:42:45 +0100
Subject: [PATCH] build: Speed up .go compilation.

* build-aux/compile-all.scm: New file.
* Makefile.am (EXTRA_DIST): Add it.
(%.go, make-go): New rules.
---
 Makefile.am               | 22 +++++--------
 build-aux/compile-all.scm | 82 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 14 deletions(-)
 create mode 100644 build-aux/compile-all.scm

diff --git a/Makefile.am b/Makefile.am
index 760caed..dfdc7b5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -302,6 +302,7 @@ EXTRA_DIST =						\
   CODE-OF-CONDUCT					\
   .dir-locals.el					\
   build-aux/build-self.scm				\
+  build-aux/compile-all.scm				\
   build-aux/hydra/gnu-system.scm			\
   build-aux/hydra/demo-os.scm				\
   build-aux/hydra/guix.scm				\
@@ -341,14 +342,6 @@ CLEANFILES =					\
   $(GOBJECTS)					\
   $(SCM_TESTS:tests/%.scm=%.log)
 
-AM_V_GUILEC = $(AM_V_GUILEC_$(V))
-AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
-AM_V_GUILEC_0 = @echo "  GUILEC" $@;
-
-# Flags passed to 'guild compile'.
-GUILD_COMPILE_FLAGS =				\
-  -Wformat -Wunbound-variable -Warity-mismatch
-
 # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
 # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
 # there that are newer than the local .scm files (for instance because the
@@ -358,14 +351,15 @@ GUILD_COMPILE_FLAGS =				\
 #
 # XXX: Use the C locale for when Guile lacks
 # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
-.scm.go:
-	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
+%.go: make-go ; @:
+make-go: $(MODULES) guix/config.scm guix/tests.scm
 	unset GUILE_LOAD_COMPILED_PATH ;				\
 	LC_ALL=C							\
+	host=$(host) srcdir="$(top_srcdir)"				\
 	$(top_builddir)/pre-inst-env					\
-	$(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)"	\
-	  $(GUILD_COMPILE_FLAGS) --target="$(host)"			\
-	  -o "$@" "$<"
+	$(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)"		\
+	  --no-auto-compile 						\
+	  -s "$(top_srcdir)"/build-aux/compile-all.scm $^
 
 SUFFIXES = .go
 
@@ -457,6 +451,6 @@ assert-final-inputs-self-contained:
 	$(top_builddir)/pre-inst-env "$(GUILE)"				\
 	  "$(top_srcdir)/build-aux/check-final-inputs-self-contained.scm"
 
-.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go
+.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go make-go
 .PHONY: assert-no-store-file-names assert-binaries-available
 .PHONY: assert-final-inputs-self-contained
diff --git a/build-aux/compile-all.scm b/build-aux/compile-all.scm
new file mode 100644
index 0000000..924ec39
--- /dev/null
+++ b/build-aux/compile-all.scm
@@ -0,0 +1,82 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(use-modules (system base target)
+             (ice-9 match)
+             (ice-9 threads)
+             (guix build utils))
+
+(define compile-options '(format unbound-variable arity-mismatch))
+
+(define host (getenv "host"))
+
+(define srcdir (getenv "srcdir"))
+
+(define (relative-file file)
+  (if (string-prefix? (string-append srcdir "/") file)
+      (string-drop file (+ 1 (string-length srcdir)))
+      file))
+
+(define (file-mtime<? f1 f2)
+  (< (stat:mtime (stat f1))
+     (stat:mtime (stat f2))))
+
+(define (scm->go file)
+  (let* ((relative (relative-file file))
+         (without-extension (string-drop-right relative 4)))
+    (string-append without-extension ".go")))
+
+(define (file-needs-compilation? file)
+  (let ((go (scm->go file)))
+    (or (not (file-exists? go))
+        (file-mtime<? go file))))
+
+(define (file->module file)
+  (let* ((relative (relative-file file))
+         (module-path (string-drop-right relative 4)))
+    (map string->symbol
+         (string-split module-path #\/))))
+
+;;; To work around <http://bugs.gnu.org/15602> (FIXME), we want to load all
+;;; files to be compiled first.  We do this via resolve-interface so that the
+;;; top-level of each file (module) is only executed once.
+(define (load-module-file file)
+  (let ((module (file->module file)))
+    (format #t "  LOAD ~a~%" module)
+    (resolve-interface module)))
+
+(define (compile-file* file output-mutex)
+  (let ((go (scm->go file)))
+    (with-mutex output-mutex
+      (format #t "  GUILEC ~a~%" go)
+      (force-output))
+    (mkdir-p (dirname go))
+    (with-target host
+      (lambda ()
+        (compile-file file
+                      #:output-file go
+                      #:opts compile-options)))))
+
+(match (command-line)
+  ((_ . files)
+   (let ((files (filter file-needs-compilation? files)))
+     (for-each load-module-file files)
+     (let ((mutex (make-mutex)))
+       (par-for-each (lambda (file)
+                       (compile-file* file mutex))
+                     files)))))
-- 
2.6.3

Re: [PATCH] build: Speed up .go compilation.

[Ludovic Courtès]

taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> So, what speedup to you get compared to ‘make -jN’?
>
> It should have the same improvement as 'guix pull' since it does
> basically the same thing.  I measured to make things concrete, and on my
> machine the run time of 'make -j4' decreases from ~26m to ~6m.

On my 4-core machine, it takes a bit less than 2mn.

> Here the lambda cannot entirely be factored out since the 'mutex' from
> the lexical scope is needed.  I still factored out the actual code and
> pass the mutex as an argument to the procedure in a minimal lambda, as
> seen in the new patch below; I hope it's readable this way.

It is.

>> It would be awesome if you could check that ‘make distcheck’ still
>> passes, and also make sure things behave correctly when modifying just
>> one file and running ‘make’, things like that.
>
> I'm having headaches with distcheck.  Currently it bails out because I'm
> missing tex.  Debian's version is apparently too old, and Guix's version
> is huge and has been downloading for many hours now.  I'll report back
> again when I'm done with that.

OK.

> Other notable changes in this version of the patch:
>
> - I noticed guix/config.scm and guix/tests.scm are not in MODULES in
>   Makefile.am (intentionally?), so I added them to the make-go rule.

Good catch.

This is indeed intentional, because $(MODULES) goes to the distribution,
but we wouldn’t want to distribute config.scm, which is derived from
config.scm.in at configure-time; tests.scm is not in $(MODULES) because
it is not installed.

> - I removed the MKDIR_P loop in the make-go rule, and do the equivalent
>   in the Scheme code now.
>
> - The target host and top source directory are now passed to the script
>   via lowercase environment variables, which makes the code a little
>   simpler.  I hope this is stylistically fine.

Good!

> From 697950b82ea86f7b7438e586bbf4efae3e87d8f8 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:42:45 +0100
> Subject: [PATCH] build: Speed up .go compilation.
>
> * build-aux/compile-all.scm: New file.
> * Makefile.am (EXTRA_DIST): Add it.
> (%.go, make-go): New rules.

The code looks good to me.  I’ve tested it, it works fine indeed, and
it’s much faster than ‘make -j4’!

I have one concern: while running it peaked at 300 MiB resident in
‘top’, which is OK on many machines but still quite a lot.  I realize we
have the same problem with ‘guix pull’ now, but that’s not great.  Part
of the problem may be that modules are not GC’d.  But anyway, that’s a
scalability problem.

What do people think?

A lesser concern is the long command-line passed to compile-all.scm,
notably because Emacs’ compilation-mode runs regexps on each line, and
that takes time proportional to the length of the line, so that leads
Emacs to hang for a second when it sees that line.  Silly.  ;-)

Thanks!
Ludo’.

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

> I have one concern: while running it peaked at 300 MiB resident in
> ‘top’, which is OK on many machines but still quite a lot.  I realize we
> have the same problem with ‘guix pull’ now, but that’s not great.  Part
> of the problem may be that modules are not GC’d.  But anyway, that’s a
> scalability problem.
>
> What do people think?

Well, my observation while testing guix pull was that before, one would
get several Guile processes each taking up about 100 MB memory if I
remember correctly.  (As many processes as one has cores.)  So for dual-
or quad-core devices we're not making things (much) worse, but maybe
some single-core devices with little memory will now have problems.

If it becomes urgent, we could partition the files to be compiled into
subsets and use a few Guile processes to compile them.  Could actually
speed things up on multi-core devices, and would still be much faster
than one-process-per-file on single-core.

> A lesser concern is the long command-line passed to compile-all.scm,
> notably because Emacs’ compilation-mode runs regexps on each line, and
> that takes time proportional to the length of the line, so that leads
> Emacs to hang for a second when it sees that line.  Silly.  ;-)

I use M-x shell so I didn't have the slowness, but it still annoyed me
actually, if only because it's ugly.  (It's also output every time one
runs 'make', even if all .go files are up to date.)

What about silencing it, like:

make-go: $(MODULES) guix/config.scm guix/tests.scm
	@echo "Compiling Scheme modules..." ;				\
	unset GUILE_LOAD_COMPILED_PATH ;				\
	...

> Thanks!
> Ludo’.

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:
>
>> It should have the same improvement as 'guix pull' since it does
>> basically the same thing.  I measured to make things concrete, and on my
>> machine the run time of 'make -j4' decreases from ~26m to ~6m.
>
> On my 4-core machine, it takes a bit less than 2mn.

OK, this is weird.  Something I did, sadly I don't know anymore what,
seems to have lead to .go files related to Guix's source tree being
cached in my $XDG_CACHE_HOME, after which running 'make -j4' in a clean
(but not newly cloned) repo indeed took only about 3 minutes.  I noticed
from lines like:

;;; note: source file ./guix/config.scm
;;;       newer than compiled /home/taylan/.cache/guile/ccache/2.0-LE-8-2.0/home/taylan/src/guix/build/guix/config.scm.go

Could it be that the same is the case for you?

Is this actually normal and I've been missing out the whole time? :-)
Maybe something in my setup/workflow while building guix gets in the way
of proper caching.

Taylan

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

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

> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:
>
>> I'm having headaches with distcheck.  Currently it bails out because I'm
>> missing tex.  Debian's version is apparently too old, and Guix's version
>> is huge and has been downloading for many hours now.  I'll report back
>> again when I'm done with that.

Distcheck passes.

I also made sure make passes at the absence of gnutls in the
environment.  This obsoletes the hack in the Makefile that was mentioned
on IRC yesterday; here's an updated patch.


[-- Attachment #2: 0001-build-Speed-up-.go-compilation.patch --]
[-- Type: text/x-diff, Size: 6437 bytes --]

From 61eab1edf73f5af9689fb1e776474fa1921a57fe 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:42:45 +0100
Subject: [PATCH] build: Speed up .go compilation.

* build-aux/compile-all.scm: New file.
* Makefile.am (EXTRA_DIST): Add it.
(%.go, make-go): New rules.
---
 Makefile.am               | 28 ++++++----------
 build-aux/compile-all.scm | 82 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 19 deletions(-)
 create mode 100644 build-aux/compile-all.scm

diff --git a/Makefile.am b/Makefile.am
index 760caed..0b9f2b4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -152,11 +152,6 @@ endif BUILD_DAEMON_OFFLOAD
 # Internal module with test suite support.
 dist_noinst_DATA = guix/tests.scm
 
-# Because of the autoload hack in (guix build download), we must build it
-# first to avoid errors on systems where (gnutls) is unavailable.
-guix/scripts/download.go: guix/build/download.go
-guix/download.go: guix/build/download.go
-
 # Linux-Libre configurations.
 KCONFIGS =					\
   gnu/packages/linux-libre-i686.conf		\
@@ -302,6 +297,7 @@ EXTRA_DIST =						\
   CODE-OF-CONDUCT					\
   .dir-locals.el					\
   build-aux/build-self.scm				\
+  build-aux/compile-all.scm				\
   build-aux/hydra/gnu-system.scm			\
   build-aux/hydra/demo-os.scm				\
   build-aux/hydra/guix.scm				\
@@ -341,14 +337,6 @@ CLEANFILES =					\
   $(GOBJECTS)					\
   $(SCM_TESTS:tests/%.scm=%.log)
 
-AM_V_GUILEC = $(AM_V_GUILEC_$(V))
-AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
-AM_V_GUILEC_0 = @echo "  GUILEC" $@;
-
-# Flags passed to 'guild compile'.
-GUILD_COMPILE_FLAGS =				\
-  -Wformat -Wunbound-variable -Warity-mismatch
-
 # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
 # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
 # there that are newer than the local .scm files (for instance because the
@@ -358,14 +346,16 @@ GUILD_COMPILE_FLAGS =				\
 #
 # XXX: Use the C locale for when Guile lacks
 # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
-.scm.go:
-	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
+%.go: make-go ; @:
+make-go: $(MODULES) guix/config.scm guix/tests.scm
+	@echo "Compiling Scheme modules..." ;				\
 	unset GUILE_LOAD_COMPILED_PATH ;				\
 	LC_ALL=C							\
+	host=$(host) srcdir="$(top_srcdir)"				\
 	$(top_builddir)/pre-inst-env					\
-	$(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)"	\
-	  $(GUILD_COMPILE_FLAGS) --target="$(host)"			\
-	  -o "$@" "$<"
+	$(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)"		\
+	  --no-auto-compile 						\
+	  -s "$(top_srcdir)"/build-aux/compile-all.scm $^
 
 SUFFIXES = .go
 
@@ -457,6 +447,6 @@ assert-final-inputs-self-contained:
 	$(top_builddir)/pre-inst-env "$(GUILE)"				\
 	  "$(top_srcdir)/build-aux/check-final-inputs-self-contained.scm"
 
-.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go
+.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go make-go
 .PHONY: assert-no-store-file-names assert-binaries-available
 .PHONY: assert-final-inputs-self-contained
diff --git a/build-aux/compile-all.scm b/build-aux/compile-all.scm
new file mode 100644
index 0000000..924ec39
--- /dev/null
+++ b/build-aux/compile-all.scm
@@ -0,0 +1,82 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(use-modules (system base target)
+             (ice-9 match)
+             (ice-9 threads)
+             (guix build utils))
+
+(define compile-options '(format unbound-variable arity-mismatch))
+
+(define host (getenv "host"))
+
+(define srcdir (getenv "srcdir"))
+
+(define (relative-file file)
+  (if (string-prefix? (string-append srcdir "/") file)
+      (string-drop file (+ 1 (string-length srcdir)))
+      file))
+
+(define (file-mtime<? f1 f2)
+  (< (stat:mtime (stat f1))
+     (stat:mtime (stat f2))))
+
+(define (scm->go file)
+  (let* ((relative (relative-file file))
+         (without-extension (string-drop-right relative 4)))
+    (string-append without-extension ".go")))
+
+(define (file-needs-compilation? file)
+  (let ((go (scm->go file)))
+    (or (not (file-exists? go))
+        (file-mtime<? go file))))
+
+(define (file->module file)
+  (let* ((relative (relative-file file))
+         (module-path (string-drop-right relative 4)))
+    (map string->symbol
+         (string-split module-path #\/))))
+
+;;; To work around <http://bugs.gnu.org/15602> (FIXME), we want to load all
+;;; files to be compiled first.  We do this via resolve-interface so that the
+;;; top-level of each file (module) is only executed once.
+(define (load-module-file file)
+  (let ((module (file->module file)))
+    (format #t "  LOAD ~a~%" module)
+    (resolve-interface module)))
+
+(define (compile-file* file output-mutex)
+  (let ((go (scm->go file)))
+    (with-mutex output-mutex
+      (format #t "  GUILEC ~a~%" go)
+      (force-output))
+    (mkdir-p (dirname go))
+    (with-target host
+      (lambda ()
+        (compile-file file
+                      #:output-file go
+                      #:opts compile-options)))))
+
+(match (command-line)
+  ((_ . files)
+   (let ((files (filter file-needs-compilation? files)))
+     (for-each load-module-file files)
+     (let ((mutex (make-mutex)))
+       (par-for-each (lambda (file)
+                       (compile-file* file mutex))
+                     files)))))
-- 
2.6.3

Re: [PATCH] build: Speed up .go compilation.

[Mark H Weaver]

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

> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:
>
>> +  (with-target target
>> +    (lambda ()
>> +      (let ((mutex (make-mutex)))
>> +        (par-for-each
>> +         (lambda (file)
>> +           (let ((go (scm->go file)))
>> +             (unless (and (file-exists? go)
>> +                          (file-mtime<? file go))
>> +               (with-mutex mutex
>> +                 (format #t "  GUILEC ~s~%" file)
>> +                 (force-output))
>> +               (compile-file file #:output-file go #:opts compile-options)
>> +               (with-mutex mutex
>> +                 (format #t "  WROTE ~s~%" go)
>> +                 (force-output)))))
>> +         files)))))

I'm sorry I haven't been following this discussion closely, but let me
first say that the performance gains you've been able to achieve are
very exciting.  Thanks for working on this!

Unfortunately, I have concerns:

A few people mentioned on IRC that when doing these concurrent
compilations within a single process, they sometimes see warnings like
this:

;;; WARNING (module #<module () 262a990> not in submodules table)

I haven't yet investigated, but my strong suspicion is that this is due
to the fact that Guile's module system is not thread safe.  More
specifically, when a new module is created, it mutates the global
directory of modules in a way that is not thread safe.

New modules are created by 'compile-file', both for the module being
compiled and for any imported modules that haven't been previously
loaded.  Unfortunately, this means that this approach of compiling files
in multiple threads within a single guile process is not safe.  There
are likely to be random crashes and corruptions.

I suggest that for now, the best way can do safely is to adopt an
approach of running multiple Guile processes, where each one compiles
multiple files within a single thread.

    Regards,
      Mark

Re: [PATCH] build: Speed up .go compilation.

[Mathieu Lirzin]

taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes:

>> that takes time proportional to the length of the line, so that leads
>> Emacs to hang for a second when it sees that line.  Silly.  ;-)
>
> I use M-x shell so I didn't have the slowness, but it still annoyed me
> actually, if only because it's ugly.  (It's also output every time one
> runs 'make', even if all .go files are up to date.)
>
> What about silencing it, like:
>
> make-go: $(MODULES) guix/config.scm guix/tests.scm
> 	@echo "Compiling Scheme modules..." ;				\
> 	unset GUILE_LOAD_COMPILED_PATH ;				\
> 	...

It would be better to use $(AM_V_at), because this will match user
verbosity choice.  With "make V=1" user will have the actual commmand
displayed but with "make V=0" which is the default value nothing will be
displayed.

--
Mathieu Lirzin

Re: [PATCH] build: Speed up .go compilation.

[Mathieu Lirzin]

Let me say first that I found this an amazing work!

taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes:

> -AM_V_GUILEC = $(AM_V_GUILEC_$(V))
> -AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
> -AM_V_GUILEC_0 = @echo "  GUILEC" $@;
> -
> -# Flags passed to 'guild compile'.
> -GUILD_COMPILE_FLAGS =				\
> -  -Wformat -Wunbound-variable -Warity-mismatch
> -

It would make sense to me to keep compile options in Makefile.am and
pass them as a command line argument to compile-all.scm.  Maybe renaming
it GUILECFLAGS or GUILEC_FLAGS or GUILE_COMPILE_FLAGS would be better (I
think Automake won't like GUILE_CFLAGS).

>  # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
>  # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
>  # there that are newer than the local .scm files (for instance because the
> @@ -358,14 +346,16 @@ GUILD_COMPILE_FLAGS =				\
>  #
>  # XXX: Use the C locale for when Guile lacks
>  # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
   ^^^

> -.scm.go:
> -	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
> +%.go: make-go ; @:
> +make-go: $(MODULES) guix/config.scm guix/tests.scm
> +	@echo "Compiling Scheme modules..." ;				\
>  	unset GUILE_LOAD_COMPILED_PATH ;				\
>  	LC_ALL=C							\
        ^^^

This is present because (scripts compile) from "old" Guile doesn't do it
automatically.  What about copying the code from the link above in
compile-all.scm and removing this from Makefile.am ?

> +	host=$(host) srcdir="$(top_srcdir)"				\
>  	$(top_builddir)/pre-inst-env					\
> -	$(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)"	\
> -	  $(GUILD_COMPILE_FLAGS) --target="$(host)"			\
> -	  -o "$@" "$<"
> +	$(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)"		\
> +	  --no-auto-compile 						\
> +	  -s "$(top_srcdir)"/build-aux/compile-all.scm $^
>  
>  SUFFIXES = .go
>  
> @@ -457,6 +447,6 @@ assert-final-inputs-self-contained:
>  	$(top_builddir)/pre-inst-env "$(GUILE)"				\
>  	  "$(top_srcdir)/build-aux/check-final-inputs-self-contained.scm"
>  
> -.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go
> +.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go make-go
>  .PHONY: assert-no-store-file-names assert-binaries-available
>  .PHONY: assert-final-inputs-self-contained
> diff --git a/build-aux/compile-all.scm b/build-aux/compile-all.scm
> new file mode 100644
> index 0000000..924ec39
> --- /dev/null
> +++ b/build-aux/compile-all.scm
[...]
> +;;; To work around <http://bugs.gnu.org/15602> (FIXME), we want to load all
> +;;; files to be compiled first.  We do this via resolve-interface so that the
> +;;; top-level of each file (module) is only executed once.
> +(define (load-module-file file)
> +  (let ((module (file->module file)))
> +    (format #t "  LOAD ~a~%" module)
                         ^^^
> +    (resolve-interface module)))
> +
> +(define (compile-file* file output-mutex)
> +  (let ((go (scm->go file)))
> +    (with-mutex output-mutex
> +      (format #t "  GUILEC ~a~%" go)
                             ^^^
I know this was already aligned this way before, but IMO It would look
cleaner to use the same alignment as Automake default silent rules (CC,
GEN, MAKEINFO...) with 11 characters including space before ‘~A’:

  (format #t "  LOAD     ~A~%" source)
  (format #t "  GUILEC   ~A~%" source)


Thanks,

--
Mathieu Lirzin

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

Mark H Weaver <mhw@netris.org> writes:

> I'm sorry I haven't been following this discussion closely, but let me
> first say that the performance gains you've been able to achieve are
> very exciting.  Thanks for working on this!

Hi Mark, thanks for the kind words. :-)

> Unfortunately, I have concerns:
>
> A few people mentioned on IRC that when doing these concurrent
> compilations within a single process, they sometimes see warnings like
> this:
>
> ;;; WARNING (module #<module () 262a990> not in submodules table)
>
> I haven't yet investigated, but my strong suspicion is that this is due
> to the fact that Guile's module system is not thread safe.  More
> specifically, when a new module is created, it mutates the global
> directory of modules in a way that is not thread safe.
>
> New modules are created by 'compile-file', both for the module being
> compiled and for any imported modules that haven't been previously
> loaded.  Unfortunately, this means that this approach of compiling files
> in multiple threads within a single guile process is not safe.  There
> are likely to be random crashes and corruptions.
>
> I suggest that for now, the best way can do safely is to adopt an
> approach of running multiple Guile processes, where each one compiles
> multiple files within a single thread.

The latest version of the patch loads all needed modules first, in the
main thread, and then compiles the files corresponding to the modules in
parallel.  Does that still lead to mutation in the directory of modules
in the parallelized segment of the code?

>     Regards,
>       Mark

Taylan

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

Mathieu Lirzin <mthl@gnu.org> writes:

> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes:
>
>>> that takes time proportional to the length of the line, so that leads
>>> Emacs to hang for a second when it sees that line.  Silly.  ;-)
>>
>> I use M-x shell so I didn't have the slowness, but it still annoyed me
>> actually, if only because it's ugly.  (It's also output every time one
>> runs 'make', even if all .go files are up to date.)
>>
>> What about silencing it, like:
>>
>> make-go: $(MODULES) guix/config.scm guix/tests.scm
>> 	@echo "Compiling Scheme modules..." ;				\
>> 	unset GUILE_LOAD_COMPILED_PATH ;				\
>> 	...
>
> It would be better to use $(AM_V_at), because this will match user
> verbosity choice.  With "make V=1" user will have the actual commmand
> displayed but with "make V=0" which is the default value nothing will be
> displayed.

Done, thanks for the recommendation.

> --
> Mathieu Lirzin

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

Mathieu Lirzin <mthl@gnu.org> writes:

> Let me say first that I found this an amazing work!

Thanks. :-)

> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes:
>
>> -AM_V_GUILEC = $(AM_V_GUILEC_$(V))
>> -AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
>> -AM_V_GUILEC_0 = @echo "  GUILEC" $@;
>> -
>> -# Flags passed to 'guild compile'.
>> -GUILD_COMPILE_FLAGS =				\
>> -  -Wformat -Wunbound-variable -Warity-mismatch
>> -
>
> It would make sense to me to keep compile options in Makefile.am and
> pass them as a command line argument to compile-all.scm.  Maybe renaming
> it GUILECFLAGS or GUILEC_FLAGS or GUILE_COMPILE_FLAGS would be better (I
> think Automake won't like GUILE_CFLAGS).

Hmm, the flags are a list of symbols in the Scheme code.  This has no
obvious representation in the Makefile.  I guess the closest is a
whitespace separated list of strings that would be turned into a list of
symbols.  Do you think it's worth this added complexity, or is there a
better way?

>>  # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
>>  # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
>>  # there that are newer than the local .scm files (for instance because the
>> @@ -358,14 +346,16 @@ GUILD_COMPILE_FLAGS =				\
>>  #
>>  # XXX: Use the C locale for when Guile lacks
>>  # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
>    ^^^
>
>> -.scm.go:
>> -	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
>> +%.go: make-go ; @:
>> +make-go: $(MODULES) guix/config.scm guix/tests.scm
>> +	@echo "Compiling Scheme modules..." ;				\
>>  	unset GUILE_LOAD_COMPILED_PATH ;				\
>>  	LC_ALL=C							\
>         ^^^
>
> This is present because (scripts compile) from "old" Guile doesn't do it
> automatically.  What about copying the code from the link above in
> compile-all.scm and removing this from Makefile.am ?

I should be using the whole (catch ...) expression, right?  Done, thanks
for the heads up.

>> +;;; To work around <http://bugs.gnu.org/15602> (FIXME), we want to load all
>> +;;; files to be compiled first.  We do this via resolve-interface so that the
>> +;;; top-level of each file (module) is only executed once.
>> +(define (load-module-file file)
>> +  (let ((module (file->module file)))
>> +    (format #t "  LOAD ~a~%" module)
>                          ^^^
>> +    (resolve-interface module)))
>> +
>> +(define (compile-file* file output-mutex)
>> +  (let ((go (scm->go file)))
>> +    (with-mutex output-mutex
>> +      (format #t "  GUILEC ~a~%" go)
>                              ^^^
> I know this was already aligned this way before, but IMO It would look
> cleaner to use the same alignment as Automake default silent rules (CC,
> GEN, MAKEINFO...) with 11 characters including space before ‘~A’:
>
>   (format #t "  LOAD     ~A~%" source)
>   (format #t "  GUILEC   ~A~%" source)

Done.

> Thanks,

Thanks for looking over it!

> --
> Mathieu Lirzin

Taylan

Re: [PATCH] build: Speed up .go compilation.

[Mathieu Lirzin]

taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes:

> Mathieu Lirzin <mthl@gnu.org> writes:
>
>> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") writes:
>>
>>> -AM_V_GUILEC = $(AM_V_GUILEC_$(V))
>>> -AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
>>> -AM_V_GUILEC_0 = @echo "  GUILEC" $@;
>>> -
>>> -# Flags passed to 'guild compile'.
>>> -GUILD_COMPILE_FLAGS =				\
>>> -  -Wformat -Wunbound-variable -Warity-mismatch
>>> -
>>
>> It would make sense to me to keep compile options in Makefile.am and
>> pass them as a command line argument to compile-all.scm.  Maybe renaming
>> it GUILECFLAGS or GUILEC_FLAGS or GUILE_COMPILE_FLAGS would be better (I
>> think Automake won't like GUILE_CFLAGS).
>
> Hmm, the flags are a list of symbols in the Scheme code.  This has no
> obvious representation in the Makefile.  I guess the closest is a
> whitespace separated list of strings that would be turned into a list of
> symbols.  Do you think it's worth this added complexity, or is there a
> better way?

I overlooked that what I proposed would require (ice-9 getopt-long).
Please ignore this suggestion, I think it would only make sense to
implement this later if we want a generic version of compile-all.scm.

>>>  # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
>>>  # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
>>>  # there that are newer than the local .scm files (for instance because the
>>> @@ -358,14 +346,16 @@ GUILD_COMPILE_FLAGS =				\
>>>  #
>>>  # XXX: Use the C locale for when Guile lacks
>>>  # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
>>    ^^^
>>
>>> -.scm.go:
>>> -	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
>>> +%.go: make-go ; @:
>>> +make-go: $(MODULES) guix/config.scm guix/tests.scm
>>> +	@echo "Compiling Scheme modules..." ;				\
>>>  	unset GUILE_LOAD_COMPILED_PATH ;				\
>>>  	LC_ALL=C							\
>>         ^^^
>>
>> This is present because (scripts compile) from "old" Guile doesn't do it
>> automatically.  What about copying the code from the link above in
>> compile-all.scm and removing this from Makefile.am ?
>
> I should be using the whole (catch ...) expression, right?  Done, thanks
> for the heads up.

Yes I suppose.  Maybe Ludo can confirm?

--
Mathieu Lirzin

Re: [PATCH] build: Speed up .go compilation.

[Ludovic Courtès]

Mathieu Lirzin <mthl@gnu.org> skribis:

>>>>  # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
>>>>  # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
>>>>  # there that are newer than the local .scm files (for instance because the
>>>> @@ -358,14 +346,16 @@ GUILD_COMPILE_FLAGS =				\
>>>>  #
>>>>  # XXX: Use the C locale for when Guile lacks
>>>>  # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
>>>    ^^^
>>>
>>>> -.scm.go:
>>>> -	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
>>>> +%.go: make-go ; @:
>>>> +make-go: $(MODULES) guix/config.scm guix/tests.scm
>>>> +	@echo "Compiling Scheme modules..." ;				\
>>>>  	unset GUILE_LOAD_COMPILED_PATH ;				\
>>>>  	LC_ALL=C							\
>>>         ^^^
>>>
>>> This is present because (scripts compile) from "old" Guile doesn't do it
>>> automatically.  What about copying the code from the link above in
>>> compile-all.scm and removing this from Makefile.am ?
>>
>> I should be using the whole (catch ...) expression, right?  Done, thanks
>> for the heads up.
>
> Yes I suppose.  Maybe Ludo can confirm?

It’s unnecessary to even call ‘setlocale’ in compile-all.scm because we
don’t rely on anything locale-specific.  So there’s no problem.

The LC_ALL=C line can also be removed from Makefile.am.

Ludo’.

Re: [PATCH] build: Speed up .go compilation.

[Ludovic Courtès]

Mark H Weaver <mhw@netris.org> skribis:

> I haven't yet investigated, but my strong suspicion is that this is due
> to the fact that Guile's module system is not thread safe.  More
> specifically, when a new module is created, it mutates the global
> directory of modules in a way that is not thread safe.
>
> New modules are created by 'compile-file', both for the module being
> compiled and for any imported modules that haven't been previously
> loaded.  Unfortunately, this means that this approach of compiling files
> in multiple threads within a single guile process is not safe.  There
> are likely to be random crashes and corruptions.

Right.  This is one of the concerns I raised before I forgot again.  ;-)

  https://lists.gnu.org/archive/html/guix-devel/2015-11/msg00359.html

Taylan writes:

> The latest version of the patch loads all needed modules first, in the
> main thread, and then compiles the files corresponding to the modules in
> parallel.  Does that still lead to mutation in the directory of modules
> in the parallelized segment of the code?

It seems to be safe, but we’re treading in a risky zone.

We’re pushing ‘compile-file’ and related code to its limits.  It would
be great to address these issues in Guile itself.

Ludo’.

Re: [PATCH] build: Speed up .go compilation.

[Ludovic Courtès]

taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:

> Well, my observation while testing guix pull was that before, one would
> get several Guile processes each taking up about 100 MB memory if I
> remember correctly.  (As many processes as one has cores.)  So for dual-
> or quad-core devices we're not making things (much) worse, but maybe
> some single-core devices with little memory will now have problems.

Right, but now the amount of memory consumed is proportional to the
number of modules compiled, whereas it was constant before.

> If it becomes urgent, we could partition the files to be compiled into
> subsets and use a few Guile processes to compile them.  Could actually
> speed things up on multi-core devices, and would still be much faster
> than one-process-per-file on single-core.

Yes.

Ludo’.

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

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

> Mathieu Lirzin <mthl@gnu.org> skribis:
>
>>>>>  # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
>>>>>  # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
>>>>>  # there that are newer than the local .scm files (for instance because the
>>>>> @@ -358,14 +346,16 @@ GUILD_COMPILE_FLAGS =				\
>>>>>  #
>>>>>  # XXX: Use the C locale for when Guile lacks
>>>>>  # <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
>>>>    ^^^
>>>>
>>>>> -.scm.go:
>>>>> -	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
>>>>> +%.go: make-go ; @:
>>>>> +make-go: $(MODULES) guix/config.scm guix/tests.scm
>>>>> +	@echo "Compiling Scheme modules..." ;				\
>>>>>  	unset GUILE_LOAD_COMPILED_PATH ;				\
>>>>>  	LC_ALL=C							\
>>>>         ^^^
>>>>
>>>> This is present because (scripts compile) from "old" Guile doesn't do it
>>>> automatically.  What about copying the code from the link above in
>>>> compile-all.scm and removing this from Makefile.am ?
>>>
>>> I should be using the whole (catch ...) expression, right?  Done, thanks
>>> for the heads up.
>>
>> Yes I suppose.  Maybe Ludo can confirm?
>
> It’s unnecessary to even call ‘setlocale’ in compile-all.scm because we
> don’t rely on anything locale-specific.  So there’s no problem.
>
> The LC_ALL=C line can also be removed from Makefile.am.

In light of this and the other changes, here's a new patch:


[-- Attachment #2: 0001-build-Speed-up-.go-compilation.patch --]
[-- Type: text/x-diff, Size: 6581 bytes --]

From 21cb57ce693467faee3ed2dfd48d7676f7d58fd5 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:42:45 +0100
Subject: [PATCH] build: Speed up .go compilation.

* build-aux/compile-all.scm: New file.
* Makefile.am (EXTRA_DIST): Add it.
(%.go, make-go): New rules.
---
 Makefile.am               | 32 ++++++------------
 build-aux/compile-all.scm | 82 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 23 deletions(-)
 create mode 100644 build-aux/compile-all.scm

diff --git a/Makefile.am b/Makefile.am
index 760caed..dd98564 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -152,11 +152,6 @@ endif BUILD_DAEMON_OFFLOAD
 # Internal module with test suite support.
 dist_noinst_DATA = guix/tests.scm
 
-# Because of the autoload hack in (guix build download), we must build it
-# first to avoid errors on systems where (gnutls) is unavailable.
-guix/scripts/download.go: guix/build/download.go
-guix/download.go: guix/build/download.go
-
 # Linux-Libre configurations.
 KCONFIGS =					\
   gnu/packages/linux-libre-i686.conf		\
@@ -302,6 +297,7 @@ EXTRA_DIST =						\
   CODE-OF-CONDUCT					\
   .dir-locals.el					\
   build-aux/build-self.scm				\
+  build-aux/compile-all.scm				\
   build-aux/hydra/gnu-system.scm			\
   build-aux/hydra/demo-os.scm				\
   build-aux/hydra/guix.scm				\
@@ -341,31 +337,21 @@ CLEANFILES =					\
   $(GOBJECTS)					\
   $(SCM_TESTS:tests/%.scm=%.log)
 
-AM_V_GUILEC = $(AM_V_GUILEC_$(V))
-AM_V_GUILEC_ = $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY))
-AM_V_GUILEC_0 = @echo "  GUILEC" $@;
-
-# Flags passed to 'guild compile'.
-GUILD_COMPILE_FLAGS =				\
-  -Wformat -Wunbound-variable -Warity-mismatch
-
 # Unset 'GUILE_LOAD_COMPILED_PATH' altogether while compiling.  Otherwise, if
 # $GUILE_LOAD_COMPILED_PATH contains $(moduledir), we may find .go files in
 # there that are newer than the local .scm files (for instance because the
 # user ran 'make install' recently).  When that happens, we end up loading
 # those previously-installed .go files, which may be stale, thereby breaking
 # the whole thing.
-#
-# XXX: Use the C locale for when Guile lacks
-# <http://git.sv.gnu.org/cgit/guile.git/commit/?h=stable-2.0&id=e2c6bf3866d1186c60bacfbd4fe5037087ee5e3f>.
-.scm.go:
-	$(AM_V_GUILEC)$(MKDIR_P) `dirname "$@"` ;			\
+%.go: make-go ; @:
+make-go: $(MODULES) guix/config.scm guix/tests.scm
+	$(AM_V_at)echo "Compiling Scheme modules..." ;			\
 	unset GUILE_LOAD_COMPILED_PATH ;				\
-	LC_ALL=C							\
+	host=$(host) srcdir="$(top_srcdir)"				\
 	$(top_builddir)/pre-inst-env					\
-	$(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)"	\
-	  $(GUILD_COMPILE_FLAGS) --target="$(host)"			\
-	  -o "$@" "$<"
+	$(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)"		\
+	  --no-auto-compile 						\
+	  -s "$(top_srcdir)"/build-aux/compile-all.scm $^
 
 SUFFIXES = .go
 
@@ -457,6 +443,6 @@ assert-final-inputs-self-contained:
 	$(top_builddir)/pre-inst-env "$(GUILE)"				\
 	  "$(top_srcdir)/build-aux/check-final-inputs-self-contained.scm"
 
-.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go
+.PHONY: sync-descriptions gen-ChangeLog gen-AUTHORS clean-go make-go
 .PHONY: assert-no-store-file-names assert-binaries-available
 .PHONY: assert-final-inputs-self-contained
diff --git a/build-aux/compile-all.scm b/build-aux/compile-all.scm
new file mode 100644
index 0000000..e0877db
--- /dev/null
+++ b/build-aux/compile-all.scm
@@ -0,0 +1,82 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(use-modules (system base target)
+             (ice-9 match)
+             (ice-9 threads)
+             (guix build utils))
+
+(define compile-options '(format unbound-variable arity-mismatch))
+
+(define host (getenv "host"))
+
+(define srcdir (getenv "srcdir"))
+
+(define (relative-file file)
+  (if (string-prefix? (string-append srcdir "/") file)
+      (string-drop file (+ 1 (string-length srcdir)))
+      file))
+
+(define (file-mtime<? f1 f2)
+  (< (stat:mtime (stat f1))
+     (stat:mtime (stat f2))))
+
+(define (scm->go file)
+  (let* ((relative (relative-file file))
+         (without-extension (string-drop-right relative 4)))
+    (string-append without-extension ".go")))
+
+(define (file-needs-compilation? file)
+  (let ((go (scm->go file)))
+    (or (not (file-exists? go))
+        (file-mtime<? go file))))
+
+(define (file->module file)
+  (let* ((relative (relative-file file))
+         (module-path (string-drop-right relative 4)))
+    (map string->symbol
+         (string-split module-path #\/))))
+
+;;; To work around <http://bugs.gnu.org/15602> (FIXME), we want to load all
+;;; files to be compiled first.  We do this via resolve-interface so that the
+;;; top-level of each file (module) is only executed once.
+(define (load-module-file file)
+  (let ((module (file->module file)))
+    (format #t "  LOAD     ~a~%" module)
+    (resolve-interface module)))
+
+(define (compile-file* file output-mutex)
+  (let ((go (scm->go file)))
+    (with-mutex output-mutex
+      (format #t "  GUILEC   ~a~%" go)
+      (force-output))
+    (mkdir-p (dirname go))
+    (with-target host
+      (lambda ()
+        (compile-file file
+                      #:output-file go
+                      #:opts compile-options)))))
+
+(match (command-line)
+  ((_ . files)
+   (let ((files (filter file-needs-compilation? files)))
+     (for-each load-module-file files)
+     (let ((mutex (make-mutex)))
+       (par-for-each (lambda (file)
+                       (compile-file* file mutex))
+                     files)))))
-- 
2.6.3

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

> Mark H Weaver <mhw@netris.org> skribis:
>
>> I haven't yet investigated, but my strong suspicion is that this is due
>> to the fact that Guile's module system is not thread safe.  More
>> specifically, when a new module is created, it mutates the global
>> directory of modules in a way that is not thread safe.
>>
>> New modules are created by 'compile-file', both for the module being
>> compiled and for any imported modules that haven't been previously
>> loaded.  Unfortunately, this means that this approach of compiling files
>> in multiple threads within a single guile process is not safe.  There
>> are likely to be random crashes and corruptions.
>
> Right.  This is one of the concerns I raised before I forgot again.  ;-)
>
>   https://lists.gnu.org/archive/html/guix-devel/2015-11/msg00359.html
>
> Taylan writes:
>
>> The latest version of the patch loads all needed modules first, in the
>> main thread, and then compiles the files corresponding to the modules in
>> parallel.  Does that still lead to mutation in the directory of modules
>> in the parallelized segment of the code?
>
> It seems to be safe, but we’re treading in a risky zone.
>
> We’re pushing ‘compile-file’ and related code to its limits.  It would
> be great to address these issues in Guile itself.

If we don't call compile-file in parallel but still do everything in one
process, we will still be cutting down the time significantly.

For instance, currently the load phase and compile phase take about one
minute each on my machine with 4 cores.  Making it non-parallel would
mean the compile time takes 4x longer, i.e. 4 min., which brings us to 5
minutes total.  The original was around 20.  On devices with fewer
cores, the parallel compile is even less important.

So in the worst case we can just turn that par-for-each into a for-each
and still get benefits.

Taylan

Re: [PATCH] build: Speed up .go compilation.

[Ludovic Courtès]

taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Mark H Weaver <mhw@netris.org> skribis:
>>
>>> I haven't yet investigated, but my strong suspicion is that this is due
>>> to the fact that Guile's module system is not thread safe.  More
>>> specifically, when a new module is created, it mutates the global
>>> directory of modules in a way that is not thread safe.
>>>
>>> New modules are created by 'compile-file', both for the module being
>>> compiled and for any imported modules that haven't been previously
>>> loaded.  Unfortunately, this means that this approach of compiling files
>>> in multiple threads within a single guile process is not safe.  There
>>> are likely to be random crashes and corruptions.
>>
>> Right.  This is one of the concerns I raised before I forgot again.  ;-)
>>
>>   https://lists.gnu.org/archive/html/guix-devel/2015-11/msg00359.html
>>
>> Taylan writes:
>>
>>> The latest version of the patch loads all needed modules first, in the
>>> main thread, and then compiles the files corresponding to the modules in
>>> parallel.  Does that still lead to mutation in the directory of modules
>>> in the parallelized segment of the code?
>>
>> It seems to be safe, but we’re treading in a risky zone.
>>
>> We’re pushing ‘compile-file’ and related code to its limits.  It would
>> be great to address these issues in Guile itself.
>
> If we don't call compile-file in parallel but still do everything in one
> process, we will still be cutting down the time significantly.
>
> For instance, currently the load phase and compile phase take about one
> minute each on my machine with 4 cores.  Making it non-parallel would
> mean the compile time takes 4x longer, i.e. 4 min., which brings us to 5
> minutes total.  The original was around 20.  On devices with fewer
> cores, the parallel compile is even less important.
>
> So in the worst case we can just turn that par-for-each into a for-each
> and still get benefits.

Right.

Mark, WDYT?

I would say: go for it, and let’s switch back to ‘for-each’ if/when we
have evidence of things going wrong.

I think that if things go wrong, that’ll be a hard failure (segfault or
some random run-time error) and not a silent hard-to-detect corruption.

Ludo’.

Re: [PATCH] build: Speed up .go compilation.

[Ludovic Courtès]

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

> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:

[...]

>> So in the worst case we can just turn that par-for-each into a for-each
>> and still get benefits.
>
> Right.
>
> Mark, WDYT?
>
> I would say: go for it, and let’s switch back to ‘for-each’ if/when we
> have evidence of things going wrong.

So, Taylan, OK to push the latest version of the patch (the one in
<874mejwzy2.fsf@T420.taylan> if I’m not mistaken)!

Thank you,
Ludo’.

Re: [PATCH] build: Speed up .go compilation.

[Taylan Ulrich Bayırlı/Kammer]

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

> ludo@gnu.org (Ludovic Courtès) skribis:
>
>> taylanbayirli@gmail.com (Taylan Ulrich "Bayırlı/Kammer") skribis:
>
> [...]
>
>>> So in the worst case we can just turn that par-for-each into a for-each
>>> and still get benefits.
>>
>> Right.
>>
>> Mark, WDYT?
>>
>> I would say: go for it, and let’s switch back to ‘for-each’ if/when we
>> have evidence of things going wrong.
>
> So, Taylan, OK to push the latest version of the patch (the one in
> <874mejwzy2.fsf@T420.taylan> if I’m not mistaken)!
>
> Thank you,
> Ludo’.

Done!

Taylan