unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: ludo@gnu.org (Ludovic Courtès)
To: "Taylan Ulrich \"Bayırlı/Kammer\"" <taylanbayirli@gmail.com>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] build: Speed up .go compilation.
Date: Fri, 08 Jan 2016 18:06:39 +0100	[thread overview]
Message-ID: <87ziwgf1b4.fsf@gnu.org> (raw)
In-Reply-To: <87mvsgxpef.fsf@T420.taylan> ("Taylan Ulrich \=\?utf-8\?Q\?\=5C\=22Bay\=C4\=B1rl\=C4\=B1\=2FKammer\=5C\=22\=22's\?\= message of "Fri, 08 Jan 2016 12:48:56 +0100")

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’.

  reply	other threads:[~2016-01-08 17:06 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12 16:41 [PATCH] Makefile: Speed up .go compilation Taylan Ulrich Bayırlı/Kammer
2016-01-08 11:48 ` [PATCH] build: " Taylan Ulrich Bayırlı/Kammer
2016-01-08 17:06   ` Ludovic Courtès [this message]
2016-01-09 19:38     ` Taylan Ulrich Bayırlı/Kammer
2016-01-09 21:59       ` Ludovic Courtès
2016-01-10 10:24         ` Taylan Ulrich Bayırlı/Kammer
2016-01-10 17:01           ` Mathieu Lirzin
2016-01-10 20:46             ` Taylan Ulrich Bayırlı/Kammer
2016-01-11 21:16           ` Ludovic Courtès
2016-01-10 13:34         ` Taylan Ulrich Bayırlı/Kammer
2016-01-10 15:11         ` Taylan Ulrich Bayırlı/Kammer
2016-01-10 17:27           ` Mathieu Lirzin
2016-01-10 20:52             ` Taylan Ulrich Bayırlı/Kammer
2016-01-10 21:18               ` Mathieu Lirzin
2016-01-11 21:05                 ` Ludovic Courtès
2016-01-11 21:47                   ` Taylan Ulrich Bayırlı/Kammer
2016-01-10 16:47     ` Mark H Weaver
2016-01-10 20:33       ` Taylan Ulrich Bayırlı/Kammer
2016-01-11 21:14       ` Ludovic Courtès
2016-01-11 21:56         ` Taylan Ulrich Bayırlı/Kammer
2016-01-14 14:02           ` Ludovic Courtès
2016-01-17 20:16             ` Ludovic Courtès
2016-01-18  8:05               ` Taylan Ulrich Bayırlı/Kammer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ziwgf1b4.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=guix-devel@gnu.org \
    --cc=taylanbayirli@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).