[m-rev.] for review: parallel mmc make
Peter Wang
wangp at students.csse.unimelb.edu.au
Mon Feb 12 12:52:40 AEDT 2007
On 2007-02-09, Julien Fischer <juliensf at csse.unimelb.edu.au> wrote:
>
> On Fri, 9 Feb 2007, Peter Wang wrote:
>
> >Branches: main
> >
> >Add initial support for parallel make functionality in `mmc --make', using
> >Mercury threads (not processes).
>
> How much work will be required to extend it to use processes instead
> of threads?
Probably not much work, if children don't need to communicate anything
complicated back to the parent. The code would be completely different
though.
> Do you have any performance figures for this change yet?
> asm_fast.gc vs. asm_fast.par.gc with one job vs. asm_fast.par.gc with
> multiple jobs.
I was testing mainly with hlc.par.gc, which works as expected, i.e. the
C -> object code step itself is nearly twice as fast with two
processors, but the rest of the build process is slower due to the .par
grade.
It turns out asm_fast.par.gc isn't making use of extra processors
properly. I won't go into the details (which I'm not entirely sure of
yet), but I think we might have to make spawn/3 actually create new
threads/engines in asm_fast.par.gc (as we do in hlc.par.gc), rather than
reusing the fixed set of engines that were created at the start of the
program. Those would then only be used for executing parallel
conjunctions. That makes sense to me anyway. Any objections?
> Does bootchecking with parallel --make work?
Yes.
> >Currently this is limited to running the
> >"compile target code to object code" step in parallel.
>
> I take it that this because of the way mmc --make currently computes
> its task list? (I think we discussed this at once?)
Yes, one problem is that when two threads are compiling different
modules, they may try to simultaneously create the same interface file
of one of the modules both of them import. Other than that, I assume
there are still some thread-safety issues remaining in the compiler and
standard library.
> >NEWS:
> > Announce parallel make support.
> >
> > Announce `list.split_upto'.
>
> and announce `can_spawn/0'.
>
Done.
> >@@ -578,7 +581,7 @@
> >
> >io_set_globals(Globals, !IO) :-
> > type_to_univ(Globals, UnivGlobals),
> >- io.set_globals(UnivGlobals, !IO).
> >+ globals.set_globals(UnivGlobals, !IO).
> >
>
> Can some of the unsafe_promise_uniques in this module now be deleted?
Yes.
> >+:- pred run_in_child(child_exits::in,
> >+ foldl2_pred_with_status(T, Info, io)::in(foldl2_pred_with_status),
> >+ Info::in, T::in, io::di, io::uo) is det.
> >+
> >+run_in_child(ChildExits, P, Info, T, !IO) :-
> >+ promise_equivalent_solutions [!:IO] (
> >+ spawn((pred(!.IO::di, !:IO::uo) is cc_multi :-
> >+ %
> >+ % Make sure something always gets written to ChildExits when P
> >+ % finishes, even if terminated by an exception.
> >+ %
> >+ try_io((pred(Succ::out, !.IO::di, !:IO::uo) is det :-
> >+ P(T, Succ, Info, _Info, !IO)
> >+ ), Result, !IO),
> >+ (
> >+ Result = succeeded(Success),
> >+ channel.put(ChildExits, Success, !IO)
> >+ ;
> >+ Result = exception(_),
> >+ channel.put(ChildExits, no, !IO),
> >+ rethrow(Result)
>
> Rethrowing that exception is almost certain to do something very dodgy
> if the thread is not the main thread. It might be better if parent loop
> were just informed that a child had terminated with an exception via
> some other mechanism (the channel would be the obvious one) rather
> than rethrowing the exception.
Good idea. But what should the parent do with the exception(s)? I made
the parent rethrow the first exception it gets (after all the running
children terminate).
> >Index: compiler/options.m
> >===================================================================
> >RCS file: /home/mercury1/repository/mercury/compiler/options.m,v
> >retrieving revision 1.546
> >diff -u -r1.546 options.m
> >--- compiler/options.m 19 Jan 2007 07:04:25 -0000 1.546
> >+++ compiler/options.m 22 Jan 2007 04:22:36 -0000
> >@@ -765,6 +765,7 @@
> > ; make
> > ; keep_going
> > ; rebuild
> >+ ; parallel_jobs
>
> I suggest just naming this option as `jobs' (for consistency with
> make if nothing else).
Changed, and the option to "--jobs".
> >@@ -1513,6 +1514,7 @@
> > make - bool(no),
> > keep_going - bool(no),
> > rebuild - bool(no),
> >+ parallel_jobs - int(1),
> > invoked_by_mmc_make - bool(no),
> > pre_link_command - maybe_string(no),
> > extra_init_command - maybe_string(no),
> >@@ -2293,6 +2295,7 @@
> >long_option("make", make).
> >long_option("keep-going", keep_going).
> >long_option("rebuild", rebuild).
> >+long_option("parallel-jobs", parallel_jobs).
>
> I suggest adding `-j' as an alias for this.
Done.
> >@@ -4792,10 +4856,10 @@
> >extern MercuryFile mercury_stderr;
> >extern MercuryFile mercury_stdin_binary;
> >extern MercuryFile mercury_stdout_binary;
> >-extern MercuryFile *mercury_current_text_input;
> >-extern MercuryFile *mercury_current_text_output;
> >-extern MercuryFile *mercury_current_binary_input;
> >-extern MercuryFile *mercury_current_binary_output;
> >+extern MR_Unsigned mercury_current_text_input_index;
> >+extern MR_Unsigned mercury_current_text_output_index;
> >+extern MR_Unsigned mercury_current_binary_input_index;
> >+extern MR_Unsigned mercury_current_binary_output_index;
>
> I suggest adding a new type to the runtime (a typedef for MR_Unsigned)
> that makes it more obvious that these things are thread local mutables,
> e.g. MR_ThreadLocalMutable (although that seems a little long.)
Ok, will do in a separate patch.
> >@@ -5521,6 +5642,7 @@
> > mercury_file_init(System.Console.OpenStandardOutput(),
> > null, System.Console.Out, ML_file_encoding_kind.ML_raw_binary);
> >
> >+// Note: these are set again in io.init_state.
>
> Use /* ... */ in preference to //. (We should try to maintain
> support for C compilers other than gcc - and some C compilers have not
> made much headway in supporting C99.)
The // comments appear in C# and Java code.
Partial interdiff follows.
Peter
diff -u compiler/make.util.m compiler/make.util.m
--- compiler/make.util.m 7 Feb 2007 01:20:47 -0000
+++ compiler/make.util.m 11 Feb 2007 02:35:09 -0000
@@ -299,8 +299,15 @@
:- import_module exception.
:- import_module thread.
:- import_module thread.channel.
+:- import_module univ.
+:- import_module unit.
-:- type child_exits == channel(bool).
+:- type child_exit
+ ---> child_succeeded
+ ; child_failed
+ ; child_exception(univ).
+
+:- type child_exits == channel(child_exit).
%-----------------------------------------------------------------------------%
@@ -357,12 +364,12 @@
foldl2_maybe_stop_at_error_maybe_parallel(KeepGoing, MakeTarget, Targets,
Success, !Info, !IO) :-
- globals.io_lookup_int_option(parallel_jobs, ParallelJobs, !IO),
+ globals.io_lookup_int_option(jobs, Jobs, !IO),
(
thread.can_spawn,
- ParallelJobs > 1
+ Jobs > 1
->
- foldl2_maybe_stop_at_error_parallel(KeepGoing, ParallelJobs,
+ foldl2_maybe_stop_at_error_parallel(KeepGoing, Jobs,
MakeTarget, Targets, Success0, !.Info, !IO)
;
Success0 = yes
@@ -380,27 +387,52 @@
foldl2_pred_with_status(T, Info, io)::in(foldl2_pred_with_status),
list(T)::in, bool::out, Info::in, io::di, io::uo) is det.
-foldl2_maybe_stop_at_error_parallel(KeepGoing, ParallelJobs,
- MakeTarget, Targets, Success, Info, !IO) :-
+foldl2_maybe_stop_at_error_parallel(KeepGoing, Jobs, MakeTarget, Targets,
+ Success, Info, !IO) :-
channel.init(ChildExits, !IO),
- list.split_upto(ParallelJobs, Targets, InitialTargets, LaterTargets),
+ list.split_upto(Jobs, Targets, InitialTargets, LaterTargets),
list.foldl(run_in_child(ChildExits, MakeTarget, Info), InitialTargets,
!IO),
parent_loop(ChildExits, KeepGoing, MakeTarget, Info,
- length(Targets), LaterTargets, yes, Success, !IO).
+ length(Targets), LaterTargets, yes, Success, no, MaybeExcp, !IO),
+ %
+ % Rethrow the first of any exceptions which terminated a child thread.
+ %
+ (
+ MaybeExcp = yes(Excp),
+ rethrow(exception(Excp) : exception_result(unit))
+ ;
+ MaybeExcp = no
+ ).
:- pred parent_loop(child_exits::in, bool::in,
foldl2_pred_with_status(T, Info, io)::in(foldl2_pred_with_status),
- Info::in, int::in, list(T)::in, bool::in, bool::out, io::di, io::uo)
- is det.
+ Info::in, int::in, list(T)::in, bool::in, bool::out,
+ maybe(univ)::in, maybe(univ)::out, io::di, io::uo) is det.
parent_loop(ChildExits, KeepGoing, MakeTarget, Info, ChildrenLeft, Targets,
- !Success, !IO) :-
+ !Success, !MaybeExcp, !IO) :-
( ChildrenLeft = 0 ->
true
;
% Wait for a running child to indicate that it is finished.
- channel.take(ChildExits, NewSuccess, !IO),
+ channel.take(ChildExits, Exit, !IO),
+ (
+ Exit = child_succeeded,
+ NewSuccess = yes
+ ;
+ Exit = child_failed,
+ NewSuccess = no
+ ;
+ Exit = child_exception(Excp),
+ (
+ !.MaybeExcp = no,
+ !:MaybeExcp = yes(Excp)
+ ;
+ !.MaybeExcp = yes(_)
+ ),
+ NewSuccess = no
+ ),
(
( NewSuccess = yes
; KeepGoing = yes
@@ -415,7 +447,7 @@
run_in_child(ChildExits, MakeTarget, Info, NextTarget, !IO)
),
parent_loop(ChildExits, KeepGoing, MakeTarget, Info,
- ChildrenLeft-1, MoreTargets, !Success, !IO)
+ ChildrenLeft-1, MoreTargets, !Success, !MaybeExcp, !IO)
;
!:Success = no
)
@@ -428,21 +460,20 @@
run_in_child(ChildExits, P, Info, T, !IO) :-
promise_equivalent_solutions [!:IO] (
spawn((pred(!.IO::di, !:IO::uo) is cc_multi :-
- %
- % Make sure something always gets written to ChildExits when P
- % finishes, even if terminated by an exception.
- %
try_io((pred(Succ::out, !.IO::di, !:IO::uo) is det :-
P(T, Succ, Info, _Info, !IO)
), Result, !IO),
(
- Result = succeeded(Success),
- channel.put(ChildExits, Success, !IO)
+ Result = succeeded(yes),
+ Exit = child_succeeded
;
- Result = exception(_),
- channel.put(ChildExits, no, !IO),
- rethrow(Result)
- )
+ Result = succeeded(no),
+ Exit = child_failed
+ ;
+ Result = exception(Excp),
+ Exit = child_exception(Excp)
+ ),
+ channel.put(ChildExits, Exit, !IO)
), !IO)
).
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------
More information about the reviews
mailing list