[m-rev.] for review: prevent interleaved errors with mmc --make

[Julien Fischer]

On Wed, 9 May 2012, Peter Wang wrote:

> Branches: main
>
> Prevent interleaved error message output when using parallel `mmc --make'.
>
> compiler/make.m:
> 	Add a field to `make_info' to hold an inter-process lock.
>
> compiler/make.util.m:
> 	Reuse the `job_ctl' type as the stdout lock.  Add predicates to acquire
> 	and release the lock.
>
> 	Make `foldl2_maybe_stop_at_error_parallel_processes' set the stdout
> 	lock in `make_info' for child processes to see.
>
> 	Acquire and release the stdout lock in various predicates that write
> 	errors messages.
>
> compiler/make.module_target.m:
> compiler/make.program_target.m:
> 	Conform to changes.
>
> diff --git a/compiler/make.m b/compiler/make.m
> index f377619..6fa5008 100644
> --- a/compiler/make.m
> +++ b/compiler/make.m
> @@ -165,7 +165,11 @@
>                 % `--analysis-repeat' and decrements to zero as analysis passes
>                 % on `suboptimal' modules are performed. `invalid' modules
>                 % are not affected as they will always be reanalysed.
> -                reanalysis_passes       :: int
> +                reanalysis_passes       :: int,
> +
> +                % An inter-process lock to prevent multiple processes
> +                % interleaving their output to standard output.
> +                maybe_stdout_lock       :: maybe(stdout_lock)
>             ).
>
> :- type module_index_map
> @@ -363,7 +367,7 @@ make_process_args(Globals, Variables, OptionArgs, Targets0, !IO) :-
>             init_cached_foreign_imports,
>             ShouldRebuildModuleDeps, KeepGoing,
>             set.init, no, set.list_to_set(ClassifiedTargets),
> -            AnalysisRepeat),
> +            AnalysisRepeat, no),
>
>         % Build the targets, stopping on any errors if `--keep-going'
>         % was not set.
> diff --git a/compiler/make.module_target.m b/compiler/make.module_target.m
> index 11ef26b..aebe927 100644
> --- a/compiler/make.module_target.m
> +++ b/compiler/make.module_target.m
> @@ -758,7 +758,7 @@ record_made_target_2(Globals, Succeeded, TargetFile, TouchedTargetFiles,
>     ;
>         Succeeded = no,
>         TargetStatus = deps_status_error,
> -        target_file_error(Globals, TargetFile, !IO)
> +        target_file_error(!.Info, Globals, TargetFile, !IO)
>     ),
>
>     list.foldl(update_target_status(TargetStatus), TouchedTargetFiles, !Info),
> diff --git a/compiler/make.program_target.m b/compiler/make.program_target.m
> index 5709442..84c18c8 100644
> --- a/compiler/make.program_target.m
> +++ b/compiler/make.program_target.m
> @@ -552,7 +552,7 @@ build_linked_target_2(Globals, MainModuleName, FileType, OutputFileName,
>     ),
>     (
>         DepsResult = deps_error,
> -        file_error(OutputFileName, !IO),
> +        file_error(!.Info, OutputFileName, !IO),
>         Succeeded = no
>     ;
>         DepsResult = deps_up_to_date,
> @@ -668,7 +668,7 @@ build_linked_target_2(Globals, MainModuleName, FileType, OutputFileName,
>                 map.delete(!.Info ^ file_timestamps, OutputFileName)
>         ;
>             Succeeded = no,
> -            file_error(OutputFileName, !IO)
> +            file_error(!.Info, OutputFileName, !IO)
>         )
>     ).
>
> diff --git a/compiler/make.util.m b/compiler/make.util.m
> index 5667c5c..5e1ac48 100644
> --- a/compiler/make.util.m
> +++ b/compiler/make.util.m
> @@ -240,6 +240,8 @@
> % Debugging, verbose and error messages
> %
>
> +:- type stdout_lock.

Add a comment describing that type.

> +
>     % Apply the given predicate if `--debug-make' is set.
>     % XXX Do we need this, now that we have trace goals?
>     %
> @@ -295,11 +297,12 @@
>
>     % Write a message "** Error making <filename>".
>     %
> -:- pred target_file_error(globals::in, target_file::in, io::di, io::uo) is det.
> +:- pred target_file_error(make_info::in, globals::in, target_file::in,
> +    io::di, io::uo) is det.
>
>     % Write a message "** Error making <filename>".
>     %
> -:- pred file_error(file_name::in, io::di, io::uo) is det.
> +:- pred file_error(make_info::in, file_name::in, io::di, io::uo) is det.
>
>     % If the given target was specified on the command line, warn that it
>     % was already up to date.
> @@ -440,9 +443,9 @@ foldl2_maybe_stop_at_error_maybe_parallel(KeepGoing, MakeTarget, Globals,
>             Targets, Success, !Info, !IO)
>     ).
>
> -:- pred foldl2_maybe_stop_at_error_parallel_processes(bool::in,
> -    int::in, foldl2_pred_with_status(T, Info, io)::in(foldl2_pred_with_status),
> -    globals::in, list(T)::in, bool::out, Info::in, Info::out,
> +:- pred foldl2_maybe_stop_at_error_parallel_processes(bool::in, int::in,
> +    foldl2_pred_with_status(T, make_info, io)::in(foldl2_pred_with_status),
> +    globals::in, list(T)::in, bool::out, make_info::in, make_info::out,
>     io::di, io::uo) is det.
>
> foldl2_maybe_stop_at_error_parallel_processes(KeepGoing, Jobs, MakeTarget,
> @@ -451,6 +454,7 @@ foldl2_maybe_stop_at_error_parallel_processes(KeepGoing, Jobs, MakeTarget,
>     create_job_ctl(TotalTasks, MaybeJobCtl, !IO),
>     (
>         MaybeJobCtl = yes(JobCtl),
> +        !Info ^ maybe_stdout_lock := yes(JobCtl),
>         list.foldl2(
>             start_worker_process(Globals, KeepGoing, MakeTarget, Targets,
>                 JobCtl, !.Info),
> @@ -460,6 +464,7 @@ foldl2_maybe_stop_at_error_parallel_processes(KeepGoing, Jobs, MakeTarget,
>             worker_loop(Globals, KeepGoing, MakeTarget, Targets, JobCtl, yes),
>             worker_loop_signal_cleanup(JobCtl, Pids), Success0, !Info, !IO),
>         list.foldl2(reap_worker_process, Pids, Success0, Success, !IO),
> +        !Info ^ maybe_stdout_lock := no,
>         destroy_job_ctl(JobCtl, !IO)
>     ;
>         MaybeJobCtl = no,
> @@ -896,6 +901,54 @@ make_yes_job_ctl(JobCtl) = yes(JobCtl).
> make_no_job_ctl = no.
>
> %-----------------------------------------------------------------------------%
> +%
> +% Prevent interleaved error output
> +%
> +
> +    % We can reuse the job_ctl type.
> +    %

Delete "can".

> +:- type stdout_lock == job_ctl.
> +
> +:- pred lock_stdout(stdout_lock::in, io::di, io::uo) is det.

Looks ok otherwise.

Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------