[m-rev.] for review: module to read trace counts

[Zoltan Somogyi]

On 11-Jan-2005, Peter Wang <wangp at students.cs.mu.OZ.AU> wrote:
> This adds a module libs__trace_counts that reads in the .mercury_trace_counts
> files produced by the compiler's trace mechanism.  The format of said files
> was slightly changed.
> 
> compiler/trace_counts.m:
> 	New module.

Since the declarative debugger also needs to be able to read trace counts,
the new module should be in the browser directory, and part of the mdbcomp
package. Any parts of the compiler, e.g. the trace_port type in trace_params.m,
that trace_counts.m depends on should also be made part of the mdbcomp package.

> runtime/mercury_trace_base.c:
> 	In the format of .mercury_trace_counts, write module and predicate
> 	names using quoted atom syntax so that names with spaces and
> 	non-printable characters can be machine-parsed.

> +% Author: wangp.
> +%
> +% This module defines a predicate to read in the execution traces generated
> +% by programs compiled using the compiler's tracing options.

An execution trace is a sequence of trace events, so trace_counts.m doesn't
read execution traces; it reads their summaries.

> +:- pred read_trace_counts(string::in, maybe(trace_counts)::out, io::di, 
> io::uo)
> +	is det.

Please keep lines down to 79 or fewer characters, here and elsewhere.

> +read_trace_counts(FileName, MaybeTraceCounts, !IO) :-
> +        io__open_input(FileName, Result, !IO),
> +        (
> +		Result = ok(FileStream),
> +                io__set_input_stream(FileStream, OldInputStream, !IO),
> +		promise_only_solution_io(read_trace_counts_2, MaybeTraceCounts,
> +			!IO),
> +                io__set_input_stream(OldInputStream, _, !IO),
> +                io__close_input(FileStream, !IO)

You should use only tabs for indentation, and make indentation consistent.

> +read_proc_trace_counts(HeaderLineNum, HeaderLine, !TraceCounts, !IO) :-
> +	lexer__string_get_token_list(HeaderLine, string__length(HeaderLine), 
> +		TokenList, posn(HeaderLineNum, 1, 0), _),
> +	(if
> +		TokenList =
> +			token_cons(name("proc"), _,
> +			token_cons(name(PredOrFuncStr), _,
> +			token_cons(name(ModuleStr), _,
> +			token_cons(name(Name), _,
> +			token_cons(integer(Arity), _,
> +			token_cons(integer(ModeInt), _,
> +			token_nil)))))),
> +		string_to_pred_or_func(PredOrFuncStr, PredOrFunc)

Using the lexer seems a bit heavyweight to me. The declarative debugger will
need to read in possibly dozens or hundreds of trace summaries, and it needs
to do so at interactive speeds. Can you tell us how long this code takes
to read in some typical sized input files?

> +            default:
> +                /* This assumes isalnum is the same as char__isalnum. */
> +                if (isalnum(*c) ||
> +                    strchr(" !@#$%^&*()-_+=`~{}[];:'\"<>.,/?\\|", *c))

Where did that string come from? It looks OK, I am just curious.

The diff is otherwise fine, but let me or Ian see another diff before you
commit.

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