guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ludovic Courtès
Subject: Re: [PATCH] build: Speed up .go compilation.
Date: Fri, 08 Jan 2016 18:06:39 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

address@hidden (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?=
>  <address@hidden>
> 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’.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]