[m-rev.] for review: deep profiling.

[Simon Taylor]

> Index: compiler/deep_profiling.m
> ===================================================================
> @@ -0,0 +1,1542 @@
> +%-----------------------------------------------------------------------------%
> +% Copyright (C) 2001 The University of Melbourne.
> +% This file may only be copied under the terms of the GNU General
> +% Public License - see the file COPYING in the Mercury distribution.
> +%-----------------------------------------------------------------------------%
> +%
> +% Main author: conway.
> +%
> +% This module applies the deep profiling transformation described in the paper
> +% ``Engineering a profiler for a logic programming language'' by Thomas Conway
> +% and Zoltan Somogyi.

A short description of what the transformation does would be useful.

> +:- pred apply_tail_recursion_to_proc(pred_proc_id::in,
> +	module_info::in, module_info::out) is det.

This predicate needs a comment.

> +maybe_transform_procedure(ModuleInfo, PredId, ProcId, ProcTable0, ProcTable,
> +		ProcStatics0, ProcStatics) :-
> +	map__lookup(ProcTable0, ProcId, ProcInfo0),
> +	proc_info_goal(ProcInfo0, Goal0),
> +	predicate_module(ModuleInfo, PredId, PredModuleName),
> +	(
> +		% XXX We need to eliminate nondet C code...
> +		Goal0 = foreign_proc(_,_,_,_,_,_, Impl) - _,
> +		Impl = nondet(_, _, _, _, _, _, _, _, _)
> +	->
> +		error("deep profiling is incompatible with nondet foreign code")

You should explain why deep profiling won't work with nondet foreign code.

> +:- pred transform_det_proc(module_info::in, pred_proc_id::in,
> +	proc_info::in, proc_info::out, maybe(layout_data)::out) is det.

transform_det_proc and transform_semi_proc should probably be combined.
The first two-thirds of each of them is identical.

> +% :- pred generate_ho_save_goal(ho_call_info, module_info, hlds_goal).
> +% :- mode generate_ho_save_goal(in, in, out) is det.
> +% 
> +% generate_ho_save_goal(ho_call_info(MiddleCSD, CountVar, PtrVar), ModuleInfo,
> +% 		Goal) :-
> +% 	generate_call(ModuleInfo, "save_and_zero_activation_info", 3,
> +% 		[MiddleCSD, CountVar, PtrVar], [CountVar, PtrVar], Goal).
> +% 
> +% generate_ho_save_goal(ho_call_info(MiddleCSD, PtrVar), ModuleInfo, Goal) :-
> +% 	generate_call(ModuleInfo, "save_and_zero_activation_info", 2,
> +% 		[MiddleCSD, PtrVar], [PtrVar], Goal).
> +% 
> +% :- pred generate_ho_restore_goal(ho_call_info, module_info,
> +% 		set(prog_var), hlds_goal).
> +% :- mode generate_ho_restore_goal(in, in, out, out) is det.
> +% 
> +% generate_ho_restore_goal(ho_call_info(MiddleCSD, CountVar, PtrVar),
> +% 		ModuleInfo, RestoreVars, Goal) :-
> +% 	RestoreVars = list_to_set([MiddleCSD, CountVar, PtrVar]),
> +% 	generate_call(ModuleInfo, "reset_activation_info", 3,
> +% 		[MiddleCSD, CountVar, PtrVar], [], Goal).
> +% 
> +% generate_ho_restore_goal(ho_call_info(MiddleCSD, PtrVar), ModuleInfo,
> +% 		RestoreVars, Goal) :-
> +% 	RestoreVars = list_to_set([MiddleCSD, PtrVar]),
> +% 	generate_call(ModuleInfo, "reset_activation_info", 2,
> +% 		[MiddleCSD, PtrVar], [], Goal).

You should explain why this code is commented out.

> Index: compiler/higher_order.m
> ===================================================================
> @@ -2248,10 +2248,20 @@
>          pred_info_arg_types(PredInfo0, ArgTVarSet, ExistQVars, Types),
>  
>  	( IsUserTypeSpec = yes ->
> -		% If this is a user-guided type specialisation, the
> -		% new name comes from the name of the requesting predicate.
> +		% If this is a user-guided type specialisation, the new name
> +		% comes from the name and mode number of the requesting
> +		% predicate. The mode number is included because we want to
> +		% avoid the creation of more than one predicate with the same
> +		% name if more than one mode of a predicate is specialized.
> +		% Since the names of e.g. deep profiling proc_static structures
> +		% are derived from the names of predicates, duplicate predicate
> +		% names lead to duplicate global variable names and hence to
> +		% link errors.
>  		Caller = proc(CallerPredId, CallerProcId),
> -		predicate_name(ModuleInfo0, CallerPredId, PredName),
> +		predicate_name(ModuleInfo0, CallerPredId, PredName0),
> +		proc_id_to_int(CallerProcId, CallerProcInt),
> +		PredName = string__append_list(
> +			[PredName0, "_", int_to_string(CallerProcInt)]),
>  		SymName = qualified(PredModule, PredName),
>  		NextHOid = NextHOid0,
>  		NewProcId = CallerProcId,

Remember to remind everyone that they will need to recompile the library
directory to avoid link errors when this change is committed.

> Index: compiler/hlds_pred.m
> ===================================================================
> @@ -1366,6 +1360,37 @@
>  	--->	address_is_taken
>  	;	address_is_not_taken.
>  
> +:- type deep_profile_role
> +	--->	inner_proc(
> +			outer_proc	:: pred_proc_id
> +		)
> +	;	outer_proc(
> +			inner_proc	:: pred_proc_id
> +		).

There should probably be a comment here.

> Index: compiler/llds_out.m
> ===================================================================
> @@ -2381,6 +2440,8 @@
>  		->
>  			[]
>  		;
> +			% XXX io__write_string("const ")
> +			% []
>  			io__write_string("const ")
>  		)
>  	;

Remove the commented out code.

> Index: compiler/modules.m
> ===================================================================
> @@ -1425,11 +1426,17 @@
>  		; globals__lookup_bool_option(Globals, trace_table_io, yes)
>  		)
>  	->
> -		UseDeps = [MercuryTableBuiltin | UseDeps1]
> +		UseDeps2 = [MercuryTableBuiltin | UseDeps1]
>  	;
> -		UseDeps = UseDeps1
> +		UseDeps2 = UseDeps1
> +	),
> +	( globals__lookup_bool_option(Globals, profile_deep, yes) ->
> +		UseDeps = [MercuryProfilingBuiltin|UseDeps2]
> +	;
> +		UseDeps = UseDeps2
>  	).

[MercuryProfilingBuiltin | UseDeps2]

> Index: compiler/typecheck.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/typecheck.m,v
> retrieving revision 1.301
> diff -u -b -r1.301 typecheck.m
> --- compiler/typecheck.m	2001/04/30 10:59:07	1.301
> +++ compiler/typecheck.m	2001/05/03 06:41:38
> @@ -5938,14 +5938,11 @@
>  		prog_out__write_context(Context),
>  		io__write_string("  Try adding explicit module qualifiers.\n")
>  	; { VerboseErrors = yes } ->
> -		io__write_strings([
> -"\tYou will need to add an explicit type qualification to resolve the\n",
> -"\ttype ambiguity.\n",
> -"\tThe way to add an explicit type qualification is to use ""with_type"".\n",
> -"\tFor details see the ""Explicit type qualification"" sub-section\n",
> -"\tof the ""Data-terms"" section of the ""Syntax"" chapter\n",
> -"\tof the Mercury langauge reference manual.\n"
> -		])
> +		io__write_string("\tYou will need to add an explicit type qualification to resolve the\n"),
> +		io__write_string("\ttype ambiguity.\n"),
> +		io__write_string("\tThe way to add an explicit type qualification\n"),
> +		io__write_string("\tis to insert a call to a dummy predicate whose `:- pred'\n"),
> +		io__write_string("\tdeclaration specifies the appropriate argument types.\n")
>  	;
>  		[]
>  	).

CVS has botched the merge here.

> Index: doc/user_guide.texi
> ===================================================================

>  @node Building profiled applications
>  @section Building profiled applications
>  
>  To enable profiling, your program must be built with profiling enabled.
> +The two different profilers require different support,
> +and thus you must choose which one to enable when you build your program.

I'd suggest replacing that paragraph with:

"To build your program with profiling enabled, choose one of the
following options:"

> + at itemize @bullet
> + at item
> +To build your program with time profiling enabled for @samp{mprof},
> +pass the @samp{-p} (@samp{--profiling}) option to @samp{mmc}
> +(and also to @samp{mgnuc} and @samp{ml}, if you invoke them separately).
> + at item
> +To build your program with memory profiling enabled for @samp{mprof},
> +pass the @samp{--memory-profiling} option to @samp{mmc},
> + at samp{mgnuc} and @samp{ml}.
> + at item
> +To build your program with deep profiling enabled (for @samp{mdprof}),
> +pass the @samp{--deep-profiling} option to @samp{mmc},
> + at samp{mgnuc} and @samp{ml}.
> + at end itemize

> +The @file{server.domain.name} part should be the name of a machine
> +with the following qualifications:
> +it should have a web server running on it,
> +and it should have the @samp{mdprof} program installed
> +in its @file{/usr/lib/cgi-bin} directory.

"The @file{server.domain.name} part should be the name of a machine
with a web server running and the @samp{mdprof} program installed
in its @file{/usr/lib/cgi-bin} directory."

>  @node Profiling and shared libraries
>  @section Profiling and shared libraries
>  
> -On some operating systems, Mercury's profiling doesn't work properly
> -with shared libraries.  The symptom is errors ("map__lookup failed")
> -or warnings from @samp{mprof}.  On some systems, the problem occurs
> -because the C implementation fails to conform to the semantics
> -specified by the ISO C standard for programs that use shared
> -libraries.  For other systems, we have not been able to analyze the
> -cause of the failure (but we suspect that the cause may be the same as
> -on those systems where we have been able to analyze it).
> -
> -If you get errors or warnings from @samp{mprof}, and your program is
> -dynamically linked, try rebuilding your application statically linked,
> -e.g. by using @samp{MLFLAGS=--static} in your Mmakefile.  Another
> -work-around that sometimes works is to set the environment variable
>  @samp{LD_BIND_NOW} to a non-null value before running the program.

> +On some operating systems,
> +Mercury's profiling doesn't work properly with shared libraries.
> +The symptom is errors ("map__lookup failed") or warnings from @samp{mprof}.
> +On some systems, the problem occurs because the C implementation
> +fails to conform to the semantics specified by the ISO C standard
> +for programs that use shared libraries.
> +For other systems, we have not been able to analyze the cause of the failure
> +(but we suspect that the cause may be the same as on those systems
> +where we have been able to analyze it).
> +
> +If you get errors or warnings from @samp{mprof},
> +and your program is dynamically linked,
> +try rebuilding your application statically linked,
> +e.g. by using @samp{MLFLAGS=--static} in your Mmakefile.
> +Another work-around that sometimes works is to set the environment variable
>  @samp{LD_BIND_NOW} to a non-null value before running the program.

Please undo this formatting change.

> Index: library/array.m
> ===================================================================
> +#endif
>  MR_BEGIN_CODE
> +	/*
> +	** Unification and comparison for arrays are implemented in Mercury,
> +	** not hand-coded low-level C
> +	*/
> +
> +#ifdef	MR_DEEP_PROFILING
> +
> +/* XXX missing prepare_for_normal_call */
> +
> +#define	proc_label	mercury____Unify___array__array_1_0
> +#define proc_static	MR_proc_static_compiler_name(array, __Unify__,	\
> +				array, 1, 0)
> +#define	body_code	MR_deep_prepare_normal_call(			\
> +			  mercury____Unify___array__array_1_0, 3,	\
> +			  mercury____Unify___array__array_1_0_i5, 0);	\

The XXX comment needs more explanation.

> Index: library/exception.m
> ===================================================================
> @@ -31,6 +31,9 @@
>  :- func throw(T) = _.
>  :- mode throw(in) = out is erroneous.
>  
> +:- pred throw_string(string).
> +:- mode throw_string(in) is erroneous.
> +
>  % The following types are used by try/3 and try/5.
>  
>  :- type exception_result(T)
> @@ -592,6 +595,11 @@
>  
>  %-----------------------------------------------------------------------------%
>  
> +:- pragma export(throw_string(in), "ML_throw_string").
> +
> +throw_string(Msg) :-
> +	throw(Msg).

As the comment at the top of exception.m says, throwing exceptions
across the C interface won't work.

> Index: tools/bootcheck
> ===================================================================
> @@ -793,19 +806,25 @@
>               /bin/rm -fr $root/stage3/* < /dev/null
>               /bin/rm -fr $root/stage3/.[a-zA-Z]* < /dev/null
>  
> +             if $keep_objs
> +             then
> +                     true
> +             else
>               case "$grade" in
>                       *debug*)
> -                             # These files take up a lot of disk space,
> -                             # so we compress them. This reduces the
> -                             # probability that running the tests will
> -                             # run out of disk space, while still allowing
> -                             # the original files to be reconstructed
> +                                     # These files take up a lot of disk
> +                                     # space, so we compress them. This
> +                                     # reduces the probability that running
> +                                     # the tests will run out of disk space,
> +                                     # while still allowing the original
> +                                     # files to be reconstructed
>                               # relatively quickly.
>                               gzip $root/stage2/library/*.c
>                               gzip $root/stage2/browser/*.c
>                               gzip $root/stage2/compiler/*.c
>                               ;;
>               esac
> +             fi
>       fi
>  
>       echo "finishing stage3 at `date`"

Fix the indenting here.

> Index: trace/mercury_trace_declarative.h
> ===================================================================
> retrieving revision 1.13
> diff -u -b -r1.13 mercury_trace_declarative.h
> --- trace/mercury_trace_declarative.h	2001/04/30 12:44:53	1.13
> +++ trace/mercury_trace_declarative.h	2001/05/03 06:41:39
> @@ -11,8 +11,6 @@
>  #include "mercury_trace.h"
>  #include "mercury_trace_internal.h"
>  
> -#ifdef MR_USE_DECLARATIVE_DEBUGGER
> -
>  /*
>  ** When in declarative debugging mode, the internal debugger calls
>  ** MR_trace_decl_debug for each event.  
> @@ -45,5 +43,4 @@
>  #define MR_TRACE_STATUS_FAILED		(MR_Word) 1
>  #define MR_TRACE_STATUS_UNDECIDED	(MR_Word) 2
>  
> -#endif  /* MR_USE_DECLARATIVE_DEBUGGER */
>  #endif	/* MERCURY_TRACE_DECLARATIVE_H */

This looks like a CVS merge error.

Simon.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------