* [bug#37510] [PATCH 1/1] compile: Fix race condition on completion progress.
@ 2019-09-25 2:07 ericbavier
2019-09-26 9:28 ` Ludovic Courtès
0 siblings, 1 reply; 5+ messages in thread
From: ericbavier @ 2019-09-25 2:07 UTC (permalink / raw)
To: 37510; +Cc: Eric Bavier
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(-)
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
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [bug#37510] [PATCH 1/1] compile: Fix race condition on completion progress.
2019-09-25 2:07 [bug#37510] [PATCH 1/1] compile: Fix race condition on completion progress ericbavier
@ 2019-09-26 9:28 ` Ludovic Courtès
2019-09-26 13:42 ` Eric Bavier
0 siblings, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2019-09-26 9:28 UTC (permalink / raw)
To: ericbavier; +Cc: 37510, Eric Bavier
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’.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug#37510] [PATCH 1/1] compile: Fix race condition on completion progress.
2019-09-26 9:28 ` Ludovic Courtès
@ 2019-09-26 13:42 ` Eric Bavier
2019-09-26 20:57 ` Ludovic Courtès
0 siblings, 1 reply; 5+ messages in thread
From: Eric Bavier @ 2019-09-26 13:42 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 37510, bavier
----- 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [bug#37510] [PATCH 1/1] compile: Fix race condition on completion progress.
2019-09-26 13:42 ` Eric Bavier
@ 2019-09-26 20:57 ` Ludovic Courtès
2019-09-28 4:01 ` bug#37510: " Eric Bavier
0 siblings, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2019-09-26 20:57 UTC (permalink / raw)
To: Eric Bavier; +Cc: 37510, bavier
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’.
^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#37510: [PATCH 1/1] compile: Fix race condition on completion progress.
2019-09-26 20:57 ` Ludovic Courtès
@ 2019-09-28 4:01 ` Eric Bavier
0 siblings, 0 replies; 5+ messages in thread
From: Eric Bavier @ 2019-09-28 4:01 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 37510-done
----- 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.
--
`~Eric
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-28 4:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25 2:07 [bug#37510] [PATCH 1/1] compile: Fix race condition on completion progress ericbavier
2019-09-26 9:28 ` Ludovic Courtès
2019-09-26 13:42 ` Eric Bavier
2019-09-26 20:57 ` Ludovic Courtès
2019-09-28 4:01 ` bug#37510: " Eric Bavier
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).