[m-rev.] for review: make the deep profiler work again

Fergus Henderson fjh at cs.mu.OZ.AU
Mon Nov 18 17:52:39 AEDT 2002


On 18-Nov-2002, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> @@ -194,8 +240,7 @@
>  		% Leave the response file to be examined.
>  	;
>  		{ Debug = no },
> -		{ RmCmd = string__format("rm %s", [s(ResponseFileName)]) },
> -		io__call_system(RmCmd, _)
> +		io__remove_file(ResponseFileName, _)
>  	).

The return value there should not be ignored.
Please at least add an XXX comment.

> @@ -308,7 +353,17 @@
>  			% want to duplicate them. We also don't want to delete
>  			% the pipes we need or the startup file.
>  			unregister_file_for_cleanup(MutexFile),
> -			unregister_file_for_cleanup(WantFile)
> +			unregister_file_for_cleanup(WantFile),
> +			io__stdin_stream(StdIn),
> +			io__close_input(StdIn),
> +			io__stdout_stream(StdOut),
> +			io__close_output(StdOut),
> +			io__stderr_stream(StdErr),
> +			io__close_output(StdErr),
> +			io__current_binary_input_stream(BinaryStdIn),
> +			io__close_binary_input(BinaryStdIn),
> +			io__current_binary_output_stream(BinaryStdOut),
> +			io__close_binary_output(BinaryStdOut)

There should be a comment explaining why you close all those streams.

Have you tested that?  I don't think that will work, since io.m will
call MR_fatal_error (or throw an exception) for any attempts to close
the pre-defined standard I/O streams.

> +#ifdef	MP_DEBUG_MDPROF_SIGNAL

That configuration macro should be documented in runtime/mercury_conf_param.h.

> --- tests/general/Mmakefile	14 Nov 2002 09:46:29 -0000	1.45
> +++ tests/general/Mmakefile	16 Nov 2002 03:20:33 -0000
> @@ -96,6 +96,8 @@
>  TESTS_DIR=..
>  include $(TESTS_DIR)/Mmake.common
>  
> +MCFLAGS += --no-optimize-duplicate-calls

I don't think that is a good idea.  Maybe you included this part accidentally?

Otherwise that looks fine.

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



More information about the reviews mailing list