[m-dev.] for review: retry across I/O

[Mark Anthony BROWN]

Hi,

Sorry for the slow review -- I was sick for most of last week.
In addition to Fergus' comments, there are a few minor issues below.

Cheers,
Mark.

Zoltan Somogyi writes:

> @@ -285,10 +358,13 @@
>  	% Some of the instmap_deltas generated in this module
>  	% are pretty dodgy (especially those for if-then-elses), so 
>  	% recompute them here.

If this comment is no longer true, then why leave it there?  You should
remove the comment, or add to it to explain why the code is masked out.

> -	RecomputeAtomic = no,
> -	recompute_instmap_delta_proc(RecomputeAtomic, ProcInfo5, ProcInfo,
> -		Module1, Module2),
>  
> +	% RecomputeAtomic = no,
> +	% recompute_instmap_delta_proc(RecomputeAtomic, ProcInfo5, ProcInfo,
> +	% 	Module1, Module2),
> +	ProcInfo = ProcInfo5,
> +	Module2 = Module1,
> +
>  	pred_info_procedures(PredInfo1, ProcTable1),
>  	map__det_update(ProcTable1, ProcId, ProcInfo, ProcTable),


> @@ -281,7 +289,14 @@
>  	%		in the maybe_decl_debug field in the proc layout;
>  	%		if there are no such slots, that field will contain -1.
>  	%
> -	% stage 5:	If --use-trail is set (given or implied), allocate
> +	% stage 5:	If the procedure has io state arguments, allocate the
> +	%		next slot to hold the saved value of the io sequence
> +	%		number, for use in implementing retry. The number of
> +	%		this slot is recorded in the maybe_io_seq field
> +	%		in the proc layout; if there is no such slot, that
> +	%		field will contain -1.
> +	%
> +	% stage 6:	If --use-trail is set (given or implied), allocate
>  	%		two slots to hold the saved value of the trail pointer
>  	%		and the ticket counter at the point of the call, for
>  	%		use in implementing retry. The number of the first of

You should check that the stages are still numbered correctly -- a recent
change has already added a stage 6.

> @@ -717,6 +829,11 @@
>  
>  	% Save any type of answer in the given answer block at the given
>  	% offset.
> +:- impure pred table_save_io_state_ans(ml_answer_block::in, int::in,
> +	io__state::ui) is det.
> +
> +	% Save any type of answer in the given answer block at the given
> +	% offset.
>  :- impure pred table_save_any_ans(ml_answer_block::in, int::in, T::in) is det.

That looks like a cut'n'paste error in the comment.

>  
>  	% Restore an integer answer from the given answer block at the
> @@ -741,6 +858,11 @@
>  
>  	% Restore any type of answer from the given answer block at the
>  	% given offset.
> +:- semipure pred table_restore_io_state_ans(ml_answer_block::in, int::in,
> +	io__state::uo) is det.
> +
> +	% Restore any type of answer from the given answer block at the
> +	% given offset.
>  :- semipure pred table_restore_any_ans(ml_answer_block::in, int::in, T::out)
>  	is det.
>  

Likewise.

> Index: runtime/mercury_tabling.h
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_tabling.h,v
> retrieving revision 1.23
> diff -u -b -r1.23 mercury_tabling.h
> --- runtime/mercury_tabling.h	2000/10/11 03:00:26	1.23
> +++ runtime/mercury_tabling.h	2000/10/22 22:51:00

This file is not mentioned in the log message.

> @@ -65,7 +65,7 @@
>  ** important that all members be the same size; this is why the simple table
>  ** status field is an (unsigned) integer, not an enum.
>  **
> -** The integer field is by generic code that does not know what kind of node
> +** The integer field is for generic code that does not know what kind of node
>  ** the node will be; this means initialization. A value of zero means the node
>  ** is uninitialized; this must be true for all members. (Also, see below on
>  ** duplicate detection.)
> @@ -86,7 +86,7 @@
>  ** the chain of trie nodes given by the input arguments of the tabled subgoal,
>  ** will be overwritten by a pointer to the answer block containing the output
>  ** arguments when the goal succeeds; the MR_SIMPLETABLE_SUCCEEDED status code
> -** is used only when the goal has no outputs, and this no answer block.
> +** is used only when the goal has no outputs, and thus no answer block.
>  ** This is why MR_SIMPLETABLE_SUCCEEDED must have the highest value, and
>  ** why code looking at MR_simpletable_status must test for success with
>  ** "table->MR_simpletable_status >= MR_SIMPLETABLE_SUCCEEDED".

> Index: runtime/mercury_trace_base.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_trace_base.c,v
> retrieving revision 1.28
> diff -u -b -r1.28 mercury_trace_base.c
> --- runtime/mercury_trace_base.c	2000/10/16 01:34:03	1.28
> +++ runtime/mercury_trace_base.c	2000/10/16 02:12:42

This looks like a separate change; it is fine -- go ahead and commit this
file.

> @@ -83,10 +83,10 @@
>  
>  /*
>  ** MR_trace_from_full is a boolean that is set before every call;
> -** it states whether the caller is being fully traced, or only interface
> -** traced. If the called code is interface traced, it will generate
> -** call, exit and fail trace events only if MR_trace_from_full is true.
> -** (It will never generate internal events.) If the called code is fully
> +** it states whether the caller is being deep traced, or only shallow
> +** traced. If the called code is shallow traced, it will generate
> +** interface trace events only if MR_trace_from_full is true.
> +** (It will never generate internal events.) If the called code is deep
>  ** traced, it will always generate all trace events, external and internal,
>  ** regardless of the setting of this variable on entry.
>  **

--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------