[PATCH 1/1] compile: Fix race condition on completion progress.

DoneSubmitted by ericbavier.
Details
2 participants
  • ericbavier
  • Ludovic Courtès
Owner
unassigned
Severity
normal
E
E
ericbavier wrote on 25 Sep 2019 04:07
(address . guix-patches@gnu.org)(name . Eric Bavier)(address . bavier@member.fsf.org)
20190925020706.6034-1-ericbavier@centurylink.net
From: Eric Bavier <bavier@member.fsf.org>
This prevent a race condition where multiple compilation threads could reportthe same completion.
* guix/build/compile.scm (compile-files)<completed>: Increment in same mutexregion as the compilation is reported.

Further reading:
When compiling many scheme files, or with '-j1', this is not usually aproblem, but with multiple build jobs and a handful of scheme files to update,you may encounter unexpected output. E.g. I recently saw this from `make -j2`:
```Compiling Scheme modules...[ 25%] LOAD gnu/packages/haskell.scm;;; note: source file ./gnu/packages/haskell.scm;;; newer than compiled /home/bavier/projects/guix/gnu/packages/haskell.go;;; note: source file ./gnu/packages/haskell.scm;;; newer than compiled /home/bavier/projects/guix/gnu/packages/haskell.go[ 50%] LOAD gnu/packages/idris.scm;;; note: source file ./gnu/packages/idris.scm;;; newer than compiled /home/bavier/projects/guix/gnu/packages/idris.go;;; note: source file ./gnu/packages/idris.scm;;; newer than compiled /home/bavier/projects/guix/gnu/packages/idris.go[ 75%] GUILEC gnu/packages/haskell.go[ 75%] GUILEC gnu/packages/idris.gomake[2]: Leaving directory '/home/bavier/projects/guix'make[1]: Leaving directory '/home/bavier/projects/guix'```--- guix/build/compile.scm | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Toggle diff (27 lines)diff --git a/guix/build/compile.scm b/guix/build/compile.scmindex c127456fd0..f77e49340a 100644--- a/guix/build/compile.scm+++ b/guix/build/compile.scm@@ -173,7 +173,8 @@ files are for HOST, a GNU triplet such as \"x86_64-linux-gnu\"." (define (build file) (with-mutex progress-lock- (report-compilation file total completed))+ (report-compilation file total completed)+ (set! completed (+ 1 completed))) ;; Exit as soon as something goes wrong. (exit-on-exception@@ -185,9 +186,7 @@ files are for HOST, a GNU triplet such as \"x86_64-linux-gnu\"." #:output-file (string-append build-directory "/" (scm->go relative)) #:opts (append warning-options- (optimization-options relative)))))))- (with-mutex progress-lock- (set! completed (+ 1 completed))))+ (optimization-options relative)))))))) (with-augmented-search-path %load-path source-directory (with-augmented-search-path %load-compiled-path build-directory-- 2.23.0
L
L
Ludovic Courtès wrote on 26 Sep 2019 11:28
(address . ericbavier@centurylink.net)
87pnjnpfsz.fsf@gnu.org
Hello,
ericbavier@centurylink.net skribis:
Toggle quote (30 lines)> From: Eric Bavier <bavier@member.fsf.org>>> This prevent a race condition where multiple compilation threads could report> the same completion.>> * guix/build/compile.scm (compile-files)<completed>: Increment in same mutex> region as the compilation is reported.>>> Further reading:>> When compiling many scheme files, or with '-j1', this is not usually a> problem, but with multiple build jobs and a handful of scheme files to update,> you may encounter unexpected output. E.g. I recently saw this from `make -j2`:>> ```> Compiling Scheme modules...> [ 25%] LOAD gnu/packages/haskell.scm> ;;; note: source file ./gnu/packages/haskell.scm> ;;; newer than compiled /home/bavier/projects/guix/gnu/packages/haskell.go> ;;; note: source file ./gnu/packages/haskell.scm> ;;; newer than compiled /home/bavier/projects/guix/gnu/packages/haskell.go> [ 50%] LOAD gnu/packages/idris.scm> ;;; note: source file ./gnu/packages/idris.scm> ;;; newer than compiled /home/bavier/projects/guix/gnu/packages/idris.go> ;;; note: source file ./gnu/packages/idris.scm> ;;; newer than compiled /home/bavier/projects/guix/gnu/packages/idris.go> [ 75%] GUILEC gnu/packages/haskell.go> [ 75%] GUILEC gnu/packages/idris.go
I think it’s expected: it shows completion at the time we started tobuild these files. Compilation of haskell.scm and idris.scm started atthe same time, and at that point we had built 75% of the files. I agreeit’s confusing though. :-)
Toggle quote (9 lines)> +++ b/guix/build/compile.scm> @@ -173,7 +173,8 @@ files are for HOST, a GNU triplet such as \"x86_64-linux-gnu\"."> > (define (build file)> (with-mutex progress-lock> - (report-compilation file total completed))> + (report-compilation file total completed)> + (set! completed (+ 1 completed)))
Here ‘completed’ is incremented before the thing is even started.
Anyway, LGTM! :-)
Ludo’.
E
E
Eric Bavier wrote on 26 Sep 2019 15:42
(name . Ludovic Courtès)(address . ludo@gnu.org)
58295170.37303056.1569505365844.JavaMail.zimbra@centurylink.net
----- On Sep 26, 2019, at 4:28 AM, Ludovic Courtès ludo@gnu.org wrote:
Toggle quote (39 lines)> Hello,> > ericbavier@centurylink.net skribis:> >> From: Eric Bavier <bavier@member.fsf.org>>>>> This prevent a race condition where multiple compilation threads could report>> the same completion.>>>> * guix/build/compile.scm (compile-files)<completed>: Increment in same mutex>> region as the compilation is reported.>>>>>> Further reading:>>>> When compiling many scheme files, or with '-j1', this is not usually a>> problem, but with multiple build jobs and a handful of scheme files to update,>> you may encounter unexpected output. E.g. I recently saw this from `make -j2`:>>>> ```>> Compiling Scheme modules...>> [ 25%] LOAD gnu/packages/haskell.scm>> ;;; note: source file ./gnu/packages/haskell.scm>> ;;; newer than compiled /home/bavier/projects/guix/gnu/packages/haskell.go>> ;;; note: source file ./gnu/packages/haskell.scm>> ;;; newer than compiled /home/bavier/projects/guix/gnu/packages/haskell.go>> [ 50%] LOAD gnu/packages/idris.scm>> ;;; note: source file ./gnu/packages/idris.scm>> ;;; newer than compiled /home/bavier/projects/guix/gnu/packages/idris.go>> ;;; note: source file ./gnu/packages/idris.scm>> ;;; newer than compiled /home/bavier/projects/guix/gnu/packages/idris.go>> [ 75%] GUILEC gnu/packages/haskell.go>> [ 75%] GUILEC gnu/packages/idris.go> > I think it’s expected: it shows completion at the time we started to> build these files. Compilation of haskell.scm and idris.scm started at> the same time, and at that point we had built 75% of the files. I agree> it’s confusing though. :-)
Right. If we did indeed expect to see completion at the time we started, then I would expect to see "0%" displayed with the first "LOAD".
Toggle quote (13 lines)> >> +++ b/guix/build/compile.scm>> @@ -173,7 +173,8 @@ files are for HOST, a GNU triplet such as>> \"x86_64-linux-gnu\".">> >> (define (build file)>> (with-mutex progress-lock>> - (report-compilation file total completed))>> + (report-compilation file total completed)>> + (set! completed (+ 1 completed)))> > Here ‘completed’ is incremented before the thing is even started.
Maybe a more generic name like "progress" would be appropriate?
Toggle quote (3 lines)> > Anyway, LGTM! :-)
Thanks for reviewing.
-- `~Eric
L
L
Ludovic Courtès wrote on 26 Sep 2019 22:57
(name . Eric Bavier)(address . ericbavier@centurylink.net)
87tv8yixmb.fsf@gnu.org
Eric Bavier <ericbavier@centurylink.net> skribis:
Toggle quote (14 lines)>>> +++ b/guix/build/compile.scm>>> @@ -173,7 +173,8 @@ files are for HOST, a GNU triplet such as>>> \"x86_64-linux-gnu\".">>> >>> (define (build file)>>> (with-mutex progress-lock>>> - (report-compilation file total completed))>>> + (report-compilation file total completed)>>> + (set! completed (+ 1 completed)))>> >> Here ‘completed’ is incremented before the thing is even started.>> Maybe a more generic name like "progress" would be appropriate?
Yes, probably!
Thanks,Ludo’.
E
E
Eric Bavier wrote on 28 Sep 2019 06:01
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 37510-done@debbugs.gnu.org)
1740315284.38306958.1569643268369.JavaMail.zimbra@centurylink.net
----- On Sep 26, 2019, at 3:57 PM, Ludovic Courtès ludo@gnu.org wrote:
Toggle quote (18 lines)> Eric Bavier <ericbavier@centurylink.net> skribis:> >>>> +++ b/guix/build/compile.scm>>>> @@ -173,7 +173,8 @@ files are for HOST, a GNU triplet such as>>>> \"x86_64-linux-gnu\".">>>> >>>> (define (build file)>>>> (with-mutex progress-lock>>>> - (report-compilation file total completed))>>>> + (report-compilation file total completed)>>>> + (set! completed (+ 1 completed)))>>> >>> Here ‘completed’ is incremented before the thing is even started.>>>> Maybe a more generic name like "progress" would be appropriate?> > Yes, probably!
Ok, pushed with a rename to "progress" in commit 21391f8c83.
-- `~Eric
Closed
?
Your comment

This issue is archived.

To comment on this conversation send email to 37510@debbugs.gnu.org