[m-rev.] for review: cleaning up and documenting I/O tabling

[Fergus Henderson]

On 18-Oct-2002, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> Document I/O tabling, after cleaning it up for public use.

That looks fine to me.
There are a few minor issues (see below) but these should be
pretty easy to address.

Were you planning on committing this before or after we branch for the
new release?

This change actually looks relatively safe to me, and it would be nice
to included it.  If you have tested the change carefully, and you're
willing to promptly address any test case regressions that it causes,
then I think it is OK to commit before we branch.

----------

The new command "table_io allow" is not documented (not even in the
"developer only" section), except in the CVS log message.  It should be,
IMHO.  It could be either documented as a "developer only" option in the
user guide, or with the documentation in the user guide commented out,
or even just as a comment in the source code which implements it; but
"this is for developers only" is not sufficient documentation, IMHO.

> Index: trace/mercury_trace_internal.c
>  	} else {
>  		MR_trace_usage("developer", "table_io");
>  	}
> @@ -4996,7 +5030,7 @@
>  		{"all", "interface", "entry", NULL};
>  
>  static const char *const	MR_trace_table_io_cmd_args[] =
> -		{"stats", "start", "end", NULL};
> +		{"stats", "start", "stop", NULL};

What about "allow"?

If you are deliberately omitting that from the list for command-line
completion because it is not documented, then I think it should at
least be mentioned in a comment here.

> Index: doc/user_guide.texi
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/doc/user_guide.texi,v
> retrieving revision 1.331
> diff -u -b -r1.331 user_guide.texi
> --- doc/user_guide.texi	16 Oct 2002 08:08:50 -0000	1.331
> +++ doc/user_guide.texi	18 Oct 2002 01:56:42 -0000
> @@ -1161,6 +1161,7 @@
>  * Tracing optimized code::
>  * Mercury debugger invocation::
>  * Mercury debugger concepts::
> +* I/O tabling::
>  * Debugger commands::
>  * Declarative debugging::
>  @end menu
> @@ -1965,6 +1966,65 @@
>  @end itemize
>  @end table
>  
> + at node I/O tabling
> + at section I/O tabling
> +
> +In Mercury, predicates that want to do I/O
> +must take a di/uo pair of I/O state arguments.
> +Some of these predicates call other predicates to do I/O for them,
> +but some are @emph{I/O primitives}, i.e. they perform the I/O themselves.
> +The Mercury standard library provides a large set of these primitives,
> +and programmers can write their own through the foreign language interface.
> +An I/O action is the execution of one call to an I/O primitive.
> +
> +In debugging grades, the Mercury implementation has the ability
> +to automatically record, for every I/O action,
> +the identity of the I/O primitive involved in the action
> +and the values of all its arguments.
> +The size of the table storing this information
> +is proportional to the number of @emph{tabled} I/O actions,
> +which are the I/O actions whose details are entered into the table.
> +Therefore the tabling of I/O actions is never turned on automatically;
> +instead, users must ask for I/O tabling to start
> +with the @samp{table_io start} command in mdb.
> +
> +The purpose of I/O tabling is to enable transparent retries across I/O actions.

It would be much nicer if this section started with a short paragraph
summarizing the nature and purpose of I/O tabling.  Readers will be
skimming through this manual; they shouldn't have to read through two
long paragraphs of definitions before they get to the bit which tells
them what the option does.

There's also a problem with ordering here.  This section occurs before
the documentation of the "retry" command.  At very least there should
be a cross-reference to the documentation for the "retry" command here.
But I think it would be better to also move this section to after the
"Debugger commands" section, and to add a forward reference to it
from the documentation of the retry command.

> Index: runtime/mercury_trace_base.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_trace_base.c,v
> retrieving revision 1.47
> diff -u -b -r1.47 mercury_trace_base.c
> --- runtime/mercury_trace_base.c	11 Sep 2002 07:20:28 -0000	1.47
> +++ runtime/mercury_trace_base.c	18 Oct 2002 03:33:44 -0000
> @@ -65,6 +65,12 @@
>  MR_Unsigned		MR_io_tabling_end = 0;
>  MR_bool			MR_io_tabling_debug = MR_FALSE;
>  
> +#ifdef	MR_REQUIRE_TRACING
> +MR_bool			MR_io_tabling_allowed = MR_TRUE;
> +#else
> +MR_bool			MR_io_tabling_allowed = MR_FALSE;
> +#endif

Both occurrences of "MR_bool" should be intended two spaces.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------