auto-connect fails to parallelize independent processes

auto-connect fails to parallelize independent processes

[Olivier Dion via]

Hi,

Given the following example:
--8<---------------cut here---------------start------------->8---
process proc (with output)
  packages "coreutils"
  outputs : file output
  # {
    for i in $(seq 10); do
      date >> {{outputs}}
      sleep 1
    done
  }

workflow parallel
  processes
    auto-connect
      proc "A.out"
      proc "B.out"
--8<---------------cut here---------------end--------------->8---

We can see that both processes are completely independent, and yet they
are batched linearly.

If we re-write the workflow with `graph' instead:
--8<---------------cut here---------------start------------->8---

define A : proc "A.out"
define B : proc "B.out"

workflow parallel
  processes
    graph
      A ->
      B ->
--8<---------------cut here---------------end--------------->8---

we do get parallel execution of the processes.

-- 
Olivier Dion
oldiob.dev

Re: auto-connect fails to parallelize independent processes

[Ricardo Wurmus]

Olivier Dion via <gwl-devel@gnu.org> writes:

> We can see that both processes are completely independent, and yet they
> are batched linearly. […]

I haven’t been able to reproduce this yet.

I wrote this test and it passes:

(let* ((proc (lambda (output)
               (make-process
                (name "does-it-matter?")
                (outputs (file output))
                (procedure '()))))
       (p1 (proc "A.out"))
       (p2 (proc "B.out"))
       (wf (make-workflow
            (name "test-workflow")
            (processes
             (auto-connect p1 p2)))))
  (test-equal "auto-connect parallelizes independent processes"
    (list (list p2 p1))
    (parallel-step-execution-order
     (workflow-processes wf)
     (workflow-restrictions wf))))

The result of parallel-step-execution-order means that p2 and p1 both
belong to the first set of processes that can be executed
simultaneously.  (There is only one set of processes, so this list of p2
and p1 is the only entry in the list of steps.)

If the above is in fact an accurate transcription of your report, we
need to look at the users of workflow-run-order (which uses
parallel-step-execution-order) to locate the bug.

-- 
Ricardo

Re: auto-connect fails to parallelize independent processes

[Olivier Dion via]

On Fri, 10 Jun 2022, Ricardo Wurmus <rekado@elephly.net> wrote:
> Olivier Dion via <gwl-devel@gnu.org> writes:
>
>> We can see that both processes are completely independent, and yet they
>> are batched linearly. […]
>
> I haven’t been able to reproduce this yet.

If you run the previous example, can you see that the first timestamp of
process B is 10 seconds after first timestamp of process A?

-- 
Olivier Dion
oldiob.dev

Re: auto-connect fails to parallelize independent processes

[Olivier Dion via]

On Fri, 10 Jun 2022, Ricardo Wurmus <rekado@elephly.net> wrote:
>
> I haven’t been able to reproduce this yet.

Afte re-testing with `graph', it seems the problem is not due to
`auto-connect'.  I've probably think it was because of caching, oops.

But the following workflow still has the same problem:
--8<---------------cut here---------------start------------->8---
process proc (with output)
  packages "coreutils"
  outputs : file output
  # {
    rm -f {{outputs}}
    for i in $(seq 3); do
      date >> {{outputs}}
      sleep 1
    done
  }

define A : proc "A.out"
define B : proc "B.out"
define C : proc "C.out"

workflow parallel
  processes
    graph
      C -> A B
--8<---------------cut here---------------end--------------->8---

A and B should run in parallel, but they do not.

-- 
Olivier Dion
oldiob.dev

[PATCH 1/2] gwl/workflows: Execute independent processes in parallel

[Olivier Dion]

Processes in the same list can be executed in parallel with `par-map'.  The
results are then appended to the accumulator.
---
 gwl/workflows.scm | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gwl/workflows.scm b/gwl/workflows.scm
index 03b29e0..74a1c74 100644
--- a/gwl/workflows.scm
+++ b/gwl/workflows.scm
@@ -43,6 +43,7 @@
 
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
+  #:use-module (ice-9 threads)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-9 gnu)
   #:use-module (srfi srfi-11)
@@ -447,13 +448,12 @@ can be used in a fold over a WORKFLOW's processes."
   (lambda (item acc)
     (match item
       ((? list?)
-       (fold (lambda (process res)
-               (cons (proc process) res))
-             acc
-             ;; By reversing the order of the processes
-             ;; in STEP we keep the output order the same
-             ;; as the order of the sequential function.
-             (reverse item)))
+       (append
+        ;; By reversing the order of the processes
+        ;; in STEP we keep the output order the same
+        ;; as the order of the sequential function.
+        (par-map (cut proc <>) (reverse item))
+        acc))
       (_ (cons (proc item) acc)))))
 
 (define* (workflow-prepare workflow engine
-- 
2.36.1

[PATCH 2/2] gwl/ui: Protect format to currnt-error-port with mutex

[Olivier Dion]

When executing processes in parallel, outputs from threads can be mangled
together if the access is not exclusive.
---
 gwl/ui.scm | 59 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/gwl/ui.scm b/gwl/ui.scm
index 35bd127..339649c 100644
--- a/gwl/ui.scm
+++ b/gwl/ui.scm
@@ -19,6 +19,7 @@
   #:use-module (guix colors)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 format)
+  #:use-module (ice-9 threads)
   #:export (G_
             log-event
 
@@ -39,6 +40,8 @@
 (define %debug-color (color BOLD MAGENTA))
 (define %execute-color (color BOLD YELLOW))
 
+(define log-mutex (make-recursive-mutex))
+
 (define* (print-diagnostic-prefix prefix #:optional location
                                   #:key (colors (color)))
   "Print PREFIX as a diagnostic line prefix."
@@ -56,36 +59,38 @@
           (colorize-string prefix colors))
         identity))
 
-  (if (location? location)
-      (format (current-error-port) "~a: ~a"
-              (location-color (location->string location))
-              (prefix-color prefix))
-      (format (current-error-port) "~a"
-              (prefix-color prefix))))
+  (with-mutex log-mutex
+    (if (location? location)
+        (format (current-error-port) "~a: ~a"
+                (location-color (location->string location))
+                (prefix-color prefix))
+        (format (current-error-port) "~a"
+                (prefix-color prefix)))))
 
 (define (log-event type . message)
   (define print?
     (or (member 'all (%config 'log-events))
         (member type (%config 'log-events))))
   (when print?
-    (case type
-      ((error)
-       (print-diagnostic-prefix (G_ "error: ") #:colors %error-color))
-      ((info)
-       (print-diagnostic-prefix (G_ "info: ") #:colors %info-color))
-      ((execute)
-       (print-diagnostic-prefix (G_ "run: ") #:colors %execute-color))
-      ((cache)
-       (print-diagnostic-prefix (G_ "cache: ") #:colors %debug-color))
-      ((debug)
-       (print-diagnostic-prefix (G_ "debug: ") #:colors %debug-color))
-      ((process)
-       (print-diagnostic-prefix (G_ "process: ") #:colors %execute-color))
-      ((guix)
-       (print-diagnostic-prefix (G_ "guix: ") #:colors %execute-color))
-      (else #true))
-    (force-output (current-error-port))
-    (format (current-error-port) "~2,2f "
-            (/ (get-internal-real-time)
-               internal-time-units-per-second))
-    (apply format (current-error-port) message)))
+    (with-mutex log-mutex
+      (case type
+        ((error)
+         (print-diagnostic-prefix (G_ "error: ") #:colors %error-color))
+        ((info)
+         (print-diagnostic-prefix (G_ "info: ") #:colors %info-color))
+        ((execute)
+         (print-diagnostic-prefix (G_ "run: ") #:colors %execute-color))
+        ((cache)
+         (print-diagnostic-prefix (G_ "cache: ") #:colors %debug-color))
+        ((debug)
+         (print-diagnostic-prefix (G_ "debug: ") #:colors %debug-color))
+        ((process)
+         (print-diagnostic-prefix (G_ "process: ") #:colors %execute-color))
+        ((guix)
+         (print-diagnostic-prefix (G_ "guix: ") #:colors %execute-color))
+        (else #true))
+      (force-output (current-error-port))
+      (format (current-error-port) "~2,2f "
+              (/ (get-internal-real-time)
+                 internal-time-units-per-second))
+      (apply format (current-error-port) message))))
-- 
2.36.1

Re: [PATCH 1/2] gwl/workflows: Execute independent processes in parallel

[Ricardo Wurmus]

Olivier Dion <olivier.dion@polymtl.ca> writes:

> Processes in the same list can be executed in parallel with `par-map'.  The
> results are then appended to the accumulator.

Thanks for the patch.  I applied it.

-- 
Ricardo

Re: [PATCH 2/2] gwl/ui: Protect format to currnt-error-port with mutex

[Ricardo Wurmus]

Olivier Dion <olivier.dion@polymtl.ca> writes:

> When executing processes in parallel, outputs from threads can be mangled
> together if the access is not exclusive.

Thanks, applied!

-- 
Ricardo

Re: [PATCH 1/2] gwl/workflows: Execute independent processes in parallel

[Olivier Dion via]

On Thu, 16 Jun 2022, Ricardo Wurmus <rekado@elephly.net> wrote:
> Olivier Dion <olivier.dion@polymtl.ca> writes:
>
>> Processes in the same list can be executed in parallel with `par-map'.  The
>> results are then appended to the accumulator.
>
> Thanks for the patch.  I applied it.

I just realize that we can remove the `(reverse item)' in the patch.
Although we can't predict the order of `par-map', the reversal of the
list is unnecessary with the removal of `fold'.

-- 
Olivier Dion
oldiob.dev