| Message ID | 20190925020706.6034-1-ericbavier@centurylink.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#37510,1/1] compile: Fix race condition on completion progress. | expand |
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. :-) > +++ 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’.
----- On Sep 26, 2019, at 4:28 AM, Ludovic Courtès ludo@gnu.org wrote: > 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". > >> +++ 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? > > Anyway, LGTM! :-) Thanks for reviewing.
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! Thanks, Ludo’.
----- On Sep 26, 2019, at 3:57 PM, Ludovic Courtès ludo@gnu.org wrote: > 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.
diff --git a/guix/build/compile.scm b/guix/build/compile.scm index 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
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 make[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(-)