From mboxrd@z Thu Jan 1 00:00:00 1970 From: taylanbayirli@gmail.com (Taylan Ulrich =?utf-8?Q?Bay=C4=B1rl=C4=B1?= =?utf-8?Q?=2FKammer?=) Subject: Re: [PATCH] build: Speed up .go compilation. Date: Sat, 09 Jan 2016 20:38:33 +0100 Message-ID: <87egdqy24m.fsf@T420.taylan> References: <87lha3rx04.fsf@T420.taylan> <87mvsgxpef.fsf@T420.taylan> <87ziwgf1b4.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:57643) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHzLH-0002TU-IH for guix-devel@gnu.org; Sat, 09 Jan 2016 14:38:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHzLB-0007DF-CU for guix-devel@gnu.org; Sat, 09 Jan 2016 14:38:23 -0500 In-Reply-To: <87ziwgf1b4.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Fri, 08 Jan 2016 18:06:39 +0100") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: guix-devel@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable ludo@gnu.org (Ludovic Court=C3=A8s) writes: > So, what speedup to you get compared to =E2=80=98make -jN=E2=80=99? 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 =E2=80=98EXTRA_DIST=E2=80=99. 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 =E2=80=98match=E2=80=99 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> + (let ((module (file->module file))) >> + (format #t " LOAD ~s~%" module) >> + (resolve-interface module))))) >> + files) > > Make the =E2=80=98lambda=E2=80=99 a named top-level procedure, for clarit= y. 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-mtimego file) file)) > > and then to: > > (let ((files (filter file-needs-compilation? files))) > ;; =E2=80=A6 > ) 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> + (with-mutex mutex >> + (format #t " GUILEC ~s~%" file) >> + (force-output)) >> + (compile-file file #:output-file go #:opts compile-optio= ns) >> + (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 =E2=80=9CWROTE=E2=80=9D output. Done. > Could you send an updated patch? Attached at the bottom. > It would be awesome if you could check that =E2=80=98make distcheck=E2=80= =99 still > passes, and also make sure things behave correctly when modifying just > one file and running =E2=80=98make=E2=80=99, 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=E2=80=99. Taylan --=-=-= Content-Type: text/x-diff; charset=utf-8 Content-Disposition: inline; filename=0001-build-Speed-up-.go-compilation.patch Content-Transfer-Encoding: quoted-printable >From 697950b82ea86f7b7438e586bbf4efae3e87d8f8 Mon Sep 17 00:00:00 2001 From: =3D?UTF-8?q?Taylan=3D20Ulrich=3D20Bay=3DC4=3DB1rl=3DC4=3DB1/Kammer?=3D 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 =3D \ 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 =3D \ $(GOBJECTS) \ $(SCM_TESTS:tests/%.scm=3D%.log) =20 -AM_V_GUILEC =3D $(AM_V_GUILEC_$(V)) -AM_V_GUILEC_ =3D $(AM_V_GUILEC_$(AM_DEFAULT_VERBOSITY)) -AM_V_GUILEC_0 =3D @echo " GUILEC" $@; - -# Flags passed to 'guild compile'. -GUILD_COMPILE_FLAGS =3D \ - -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 =3D \ # # XXX: Use the C locale for when Guile lacks # . -.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=3DC \ + host=3D$(host) srcdir=3D"$(top_srcdir)" \ $(top_builddir)/pre-inst-env \ - $(GUILD) compile -L "$(top_builddir)" -L "$(top_srcdir)" \ - $(GUILD_COMPILE_FLAGS) --target=3D"$(host)" \ - -o "$@" "$<" + $(GUILE) -L "$(top_builddir)" -L "$(top_srcdir)" \ + --no-auto-compile \ + -s "$(top_srcdir)"/build-aux/compile-all.scm $^ =20 SUFFIXES =3D .go =20 @@ -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" =20 -.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 =C2=A9 2016 Taylan Ulrich Bay=C4=B1rl=C4=B1/Kammer +;;; +;;; 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 . + +(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-mtimego 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-mtimemodule file) + (let* ((relative (relative-file file)) + (module-path (string-drop-right relative 4))) + (map string->symbol + (string-split module-path #\/)))) + +;;; To work around (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))))) --=20 2.6.3 --=-=-=--