Re: [PATCHv2 1/8] run_processes_parallel: Add output to tracing messages

[Eric Sunshine <ericsunshine@gmail.com>]

On Wed, Oct 28, 2015 at 7:21 PM, Stefan Beller <sbeller@google.com> wrote:
> run_processes_parallel: Add output to tracing messages

This doesn't really say much. I guess you mean that the intention is
to delimit a section in which output from various tasks may be
intermixed. Perhaps:

    run_processes_parallel: delimit intermixed task output

or something.

> This commit serves 2 purposes. First this may help the user who
> tries to diagnose intermixed process calls. Second this may be used
> in a later patch for testing. As the output of a command should not
> change visibly except for going faster, grepping for the trace output
> seems like a viable testing strategy.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/run-command.c b/run-command.c
> index 82cc238..49dec74 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -959,6 +959,9 @@ static struct parallel_processes *pp_init(int n,
>                 n = online_cpus();
>
>         pp->max_processes = n;
> +
> +       trace_printf("run_processes_parallel: preparing to run up to %d children in parallel", n);

s/children/tasks/ maybe?

Minor: Perhaps drop "in parallel" since the parallelism is already
implied by the "run_processes_parallel" prefix.

> +
>         pp->data = data;
>         if (!get_next_task)
>                 die("BUG: you need to specify a get_next_task function");
> @@ -988,6 +991,7 @@ static void pp_cleanup(struct parallel_processes *pp)
>  {
>         int i;
>
> +       trace_printf("run_processes_parallel: parallel processing done");

Minor: Likewise, perhaps just "done" rather than "parallel processing
done" since the "run_processes_parallel" prefix already implies
parallelism.

>         for (i = 0; i < pp->max_processes; i++) {
>                 strbuf_release(&pp->children[i].err);
>                 child_process_deinit(&pp->children[i].process);
> --
> 2.5.0.281.g4ed9cdb