[m-rev.] for review: add mdb `dice' command

[Julien Fischer]

On Sun, 6 Feb 2005, Ian MacLarty wrote:

> For review by anyone.
>
> Estimated hours taken: 40
> Branches: main
>
> Add mdb `dice' command which reads in a set of passing trace counts and a
> failing trace count and prints a comparison table.  The table can be sorted
> by various metrics and is useful for finding parts of a program executed in
> a failing run, but not in passing runs.
>
The log message should mention that this eventually intendted to be used
as part of the declartive debugger.

> browser/dice.m
> 	Add a new module for generating and manipulating a dice.
>
> browser/mdb.m
> 	Add the dice module.
>
> doc/user_guide.texi
> 	Document the `dice' mdb command.  Also document the fact that the
> 	failing and passing slice file names can be set with the `set' mdb
> 	command.
>
> 	Move the `set' command to the misc section from the browser section
> 	since it now also sets the passing and failing slice file names, which
> 	have nothing to do with the browser.
>
> mdbcomp/program_representation.m
> 	Add a predicate to convert a goal path to a string.
>
> mdbcomp/trace_counts.m
> 	Convert string_to_trace_port into a predicate and add a new mode so
> 	that a port can be converted back to a string.
>
> runtime/mercury_trace_base.h
> trace/mercury_trace_util.h
> 	Move the MR_TRACE_USE_HP and MR_TRACE_CALL_MERCURY macros to
> 	runtime/mercury_trace_base.h, so that they can be called from
> 	browser/dice.m.
>
> tests/Mmake.common
> 	Clean up trace counts (which are generated to test the `dice' command).
>
> tests/debugger/Mmakefile
> tests/debugger/dice.exp
> tests/debugger/dice.inp
> tests/debugger/dice.m
> tests/debugger/dice.passes
> 	Test the `dice' command.
>
> trace/mercury_trace_internal.c
> 	Add the mdb `dice' command and modify the `set' command so the
> 	`fail_trace_count' and `pass_trace_counts' parameters can be set.
>
> 	Add a function to print a dice.
>
> 	Move the `set' command to the misc help section.
>
> Index: browser/dice.m
> ===================================================================
> RCS file: browser/dice.m
> diff -N browser/dice.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ browser/dice.m	6 Feb 2005 07:47:34 -0000
> @@ -0,0 +1,626 @@
> +%-----------------------------------------------------------------------------%
> +% Copyright (C) 1999-2005 The University of Melbourne.

You started work on this in 1999?

> +% This file may only be copied under the terms of the GNU Library General
> +% Public License - see the file COPYING.LIB in the Mercury distribution.
> +%-----------------------------------------------------------------------------%
> +% File: dice.m
> +% Author: Ian MacLarty
> +%
> +% This module contains code for generating and manipulating dices.  A dice
> +% is the difference between one or more passing test runs and one or more
> +% failing test runs.
> +%
> +

...

> +	% read_dice_to_string(PassFiles, FailFile, SortStr, N, DiceStr,
> +	% 	Problem, !IO).
> +	% Read the trace_counts in the list of files in the file named
> +	% PassFiles, interpretting them as passing slices; read the
s/interpretting/interpreting/

> +	% trace_counts from the file FailFile, interpretting it as a failing
And again here.

> +	% slice; then produce a dice and convert the dice to a string suitable
> +	% for displaying on the screen, sorting it first using SortStr.
> +	% SortStr can be any combination of the letters "sSpPfP" and
> +	% indicates how the dice is to be sorted.  See the documentation
> +	% for the `dice' command in the user guide for an explaination of the
> +	% sort string.  If there was a problem reading the trace_counts then
> +	% Problem will contain a string describing the problem encountered and
> +	% DiceStr will be the empty string, otherwise Problem will be the
> +	% empty string.
> +	%
> +:- pred read_dice_to_string(string::in, string::in, string::in, int::in,
> +	string::out, string::out, io::di, io::uo) is det.
> +
> +:- pragma export(read_dice_to_string(in, in, in, in, out, out, di, uo),
> +	"MR_MDB_read_dice_to_string").
> +
> +read_dice_to_string(PassFiles, FailFile, SortStr, N, DiceStr, Problem, !IO) :-
> +	(

...

> +	% Produce a formatted table from a list of label_counts.
> +	%
> +:- func format_label_counts(list(label_count), int, int) = string.
> +
> +format_label_counts(LabelCounts, TotalPassTests, _TotalFailTests) = Str :-
> +	dice.map6(deconstruct_label_count, LabelCounts, ProcLabels,
> +		PathPorts, PassCounts, PassTests, FailCounts, _FailTests),
> +	FormattedProcLabels = list.map(format_proc_label, ProcLabels),
> +	FormattedPathPorts = list.map(format_path_port, PathPorts),
> +	FormattedContexts = list.map(format_context, LabelCounts),
> +	PassCountStrs = list.map(string.int_to_string_thousands, PassCounts),
> +	PassTestsStrs = list.map(bracket_int, PassTests),
> +	FailCountStrs = list.map(string.int_to_string_thousands, FailCounts),
> +	SuspicionIndices = list.map_corresponding(suspicion_ratio, PassCounts,
> +		FailCounts),
> +	FormattedSuspicionIndices = list.map(dice.format_float(2),
> +		SuspicionIndices),
> +	TotalPassTestsStr = "(" ++ int_to_string_thousands(TotalPassTests)
> +		++ ")",
> +	Str = string.format_table([
> +		left( ["Procedure"       | FormattedProcLabels]),
> +		left( ["Path/Port"       | FormattedPathPorts]),
> +		left( ["File:Line"       | FormattedContexts]),
> +		right(["Pass"            | PassCountStrs]),
> +		right([TotalPassTestsStr | PassTestsStrs]),
> +		right(["Fail"            | FailCountStrs]),
> +		right(["Suspicion"       | FormattedSuspicionIndices])], " ").
> +
> +:- func bracket_int(int) = string.
> +
> +bracket_int(X) = "(" ++ string.int_to_string_thousands(X) ++ ")".
> +
> +:- func suspicion_ratio(int, int) = float.
> +
> +	% suspicion_ration gives an indication of how lightly a label is to
s/suspicion_ration/suspicion_ratio/
s/lightly/likely/

> +	% be buggy based on how many times it was executed in passing and
> +	% failing test runs.
> +	%
> +suspicion_ratio(PassCount, FailCount) =
> +	% PassCount + FailCount should never be zero since if a label
> +	% isn't executed in any tests then it wouldn't be included in the dice.
> +	float(FailCount) / float(PassCount + FailCount).
> +
I suggest putting a check that PassCount + FailCount \= 0 there.

> +:- func format_float(int, float) = string.
> +
> +format_float(DecimalPlaces, Flt) = string.format("%." ++
> +	int_to_string(DecimalPlaces) ++ "f", [f(Flt)]).
> +
You could write that more succinctly as:

	string.format("%.*f", [i(DecimalPlaces), f(Flt)])

...

> +:- func format_proc_label(proc_label) = string.
> +
> +format_proc_label(ProcLabel) = Str :-
> +	(
> +		ProcLabel = proc(_, PredOrFunc, SymModule, Name, Arity,
> +			ModeNo),
> +		Module = sym_name_to_string(SymModule),
> +		(
> +			PredOrFunc = function,
> +			ArityStr = int_to_string(Arity - 1),
> +			PredOrFuncStr = "func"
> +		;
> +			PredOrFunc = predicate,
> +			ArityStr = int_to_string(Arity),
> +			PredOrFuncStr = "pred"
> +		),
> +		Str = PredOrFuncStr ++ " " ++ Module ++ "." ++ Name ++
> +			"/" ++ ArityStr ++ "-" ++ int_to_string(ModeNo)
> +	;
> +		ProcLabel = special_proc(_, SpecialPredId, SymModule, TypeName,
> +			Arity, _),
> +		Module = sym_name_to_string(SymModule),
> +		(
> +			SpecialPredId = unify,
> +			Name = "__Unify__"
> +		;
> +			SpecialPredId = index,
> +			Name = "__Index__"
> +		;
> +			SpecialPredId = compare,
> +			Name = "__Compare__"
> +		;
> +			SpecialPredId = initialise,
> +			Name = "__Initialise__"
> +		),
The module compiler/special_pred contains predicates for handling the
names of special preds.  I suggest moving the relevant bits into the
mdbcomp library and using them here.

> +		Str = Name ++ " for " ++ Module ++ "." ++ TypeName ++ "/" ++
> +			int_to_string(Arity)
> +	).
> +

...

> +:- pred find_context_for_proc(string::in, string::in, int::in, int::in,
> +	path_port::in, string::out, int::out) is semidet.
> +
> +:- pragma foreign_proc(c, find_context_for_proc(Module::in, Name::in,
> +	Arity::in, ModeNo::in, PathPort::in, FileName::out, LineNo::out),
> +	[promise_pure, may_call_mercury, thread_safe, terminates],
> +"
> +    const MR_Module_Layout		*module;
> +    const MR_Module_File_Layout		*file;
> +    const MR_Label_Layout		*label;
> +    const MR_Proc_Layout		*proc;
> +    const MR_User_Proc_Id		*id;
> +    int					num_modules;
> +    int					module_num;
> +    int					file_num;
> +    int					num_files;
> +    int					num_labels;
> +    int					label_num;
> +    const char*				filename;
> +    MR_bool				are_same;
> +
> +    num_modules = MR_module_info_next;
> +
> +    for (module_num = 0; module_num < num_modules; module_num++) {
> +        module = MR_module_infos[module_num];
> +        if (MR_streq(Module, module->MR_ml_name)) {
> +            num_files = module->MR_ml_filename_count;
> +            for (file_num = 0; file_num < num_files; file_num++) {
> +                file = module->MR_ml_module_file_layout[file_num];
> +                num_labels = file->MR_mfl_label_count;
> +                for (label_num = 0; label_num < num_labels; label_num++) {
> +                    label = file->MR_mfl_label_layout[label_num];
> +                    proc = label->MR_sll_entry;
> +                    id = &proc->MR_sle_user;
> +                    if (MR_streq(id->MR_user_name, Name) &&
> +                            id->MR_user_arity == Arity &&
> +                            id->MR_user_mode == ModeNo) {
> +                        MR_TRACE_CALL_MERCURY(
> +                            MR_MDB_same_path_port(
> +			        (MR_String) MR_label_goal_path(label),
> +                                label->MR_sll_port, PathPort, &are_same);
> +                        );
> +                        if (are_same) {
> +                            SUCCESS_INDICATOR = MR_find_context(label,
> +                                &filename, &LineNo);
> +                            MR_TRACE_USE_HP(
> +                                MR_make_aligned_string(FileName,
> +                                    (MR_String) filename);
> +                            );
> +                            goto end;
> +                        }
> +                    }
> +                }
> +            }
> +        }
> +    }
> +
The amount of nesting and lack of whitespace here make this block of
code very difficult to understand.  Would it be possible to factor some
of this out into separate functions?

> +    SUCCESS_INDICATOR = MR_FALSE;
> +    end:
> +").
> +

...

> +:- pred map6(pred(T, T1, T2, T3, T4, T5, T6), list(T), list(T1),
> +	list(T2), list(T3), list(T4), list(T5), list(T6)).
> +:- mode map6(pred(in, out, out, out, out, out, out) is det, in, out, out,
> +	out, out, out, out) is det.
> +
> +map6(_, [], [], [], [], [], [], []).
> +map6(P, [H | T], [H1 | T1], [H2 | T2], [H3 | T3], [H4 | T4], [H5 | T5],
> +		[H6 | T6]) :-
> +	P(H, H1, H2, H3, H4, H5, H6),
> +	dice.map6(P, T, T1, T2, T3, T4, T5, T6).

You may want to consider adding that to the standard library.

> Index: doc/user_guide.texi
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/doc/user_guide.texi,v
> retrieving revision 1.416
> diff -u -r1.416 user_guide.texi
> --- doc/user_guide.texi	2 Feb 2005 02:59:06 -0000	1.416
> +++ doc/user_guide.texi	6 Feb 2005 07:52:24 -0000
> @@ -2722,90 +2722,6 @@
>  which were printed when control arrived at the event,
>  have since scrolled off the screen.
>  @sp 1

...

> + at sp 1
> + at item dice [-p at var{filename}] [-f at var{filename}] [-n at var{num}] [-s[pPfFsS]+] [-o @var{filename}]
> + at kindex dice (mdb command)
> +Display a program dice on the screen.
> + at sp 1
> +A dice is a comparison between some successful test runs of the program and a
> +failing test run.  Before using the @samp{dice} command one or more passing
> +execution summaries and one failing execution summary should be generated.
> +This can be done by compiling the program with deep tracing enabled (either by
> +compiling in the .debug or .decldebug grades or with the @samp{--trace deep}

The .decldebug grade is not currently documented.

> +compiler option) and then running the program with the MERCURY_OPTIONS
> +environment variable set to @samp{--trace-count}.
> +This will generate a file called
> + at samp{.mercury_trace_counts} which contains a summary of the program's execution
> +(called a slice).  Copy the generated slice to a new file for each test case,
> +to end up with some passing slices, say @samp{pass1}, @samp{pass2},
> + at samp{pass3}, etc. and a
> +failing slice, say @samp{fail}.
> + at sp 1
> +Once one or more passing slices and a failing slice has been generated the
s/has/have/

> + at samp{dice} command can be used to display a table of statistics comparing the
> +passing test runs to the failing run.  Each row in the table contains statistics
> +about the execution of a seperate goal in the program.  Six columns are
s/seperate/separate/

> +displayed:
> + at sp 1
> + at itemize @bullet
> + at item @samp{Procedure}:
> +The procedure in which the goal appears.
> + at item @samp{Path/Port}:
> +The goal path and/or port of the goal.  For atomic goals, statistics about the
> +CALL event and the corresponding EXIT, FAIL or EXCP event are displayed on
> +seperate rows.  For other types of goals the goal path is displayed, except for
s/seperate/separate/

More later...

Cheers,
Julien.



--------------------------------------------------------------------------
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
--------------------------------------------------------------------------