[m-rev.] for review: fix smart recompilation bug

Fergus Henderson fjh at cs.mu.OZ.AU
Wed Jul 4 05:37:06 AEST 2001


On 04-Jul-2001, Simon Taylor <stayl at cs.mu.OZ.AU> wrote:
> 
> Use a timestamp file mechanism similar to that used for `.int' files 
> to avoid rechecking whether a module needs to be recompiled on each
> mmake if no used interfaces have changed.

Hmm... your change means that this timestamp file mechanism is used
even when smart recompilation is not enabled.  This will slow down
compilation in the case when smart recompilation is not enabled.
Compilation will be slower because

	(1) More files need to be read
	(2) More files need to be written
	(3) The directory is bigger, because it contains more files

These effects will be more pronounced if file I/O is expensive,
e.g. when using NFS file systems.

Have you measured how much effect this change has on compilation
speed when compiling over NFS?

Have you considered enabling the new mechanism only when
`--smart-recompilation' is used?


The code in configure.in which does `rm -f .../*.c' will need to be
changed.

The code in compiler/modules.m which calculates foo_s_dates is wrong for
`--target asm'; but it is copied from the code which calculates foo_ss,
which is also wrong.  The way we calculate `foo_os' is the right way.

> Index: compiler/mercury_compile.m
> +find_timestamp_files(TopLevelModuleName, Globals, FindTimestampFiles) :-
> +	globals__lookup_bool_option(Globals, pic_reg, PicReg),
> +	globals__get_target(Globals, CompilationTarget),
> +	( CompilationTarget = c, TimestampSuffix = ".c_date"
> +	; CompilationTarget = il, TimestampSuffix = ".il_date"
> +	; CompilationTarget = java, TimestampSuffix = ".java_date"
> +	; CompilationTarget = asm,
> +		TimestampSuffix = (PicReg = yes -> ".pic_s_date" ; ".s_date")
> +	),
> +	FindTimestampFiles =
> +	    (pred(ModuleName::in, TimestampFiles::out, di, uo) is det -->
> +		(
> +			{ CompilationTarget = asm },
> +			{ ModuleName \= TopLevelModuleName }
> +		->
> +			% With `--target asm' all the nested
> +			% sub-modules are placed in the `.s' file
> +			% of the top-level module.
> +			{ TimestampFiles = [] }
> +		;
> +			module_name_to_file_name(ModuleName,
> +				TimestampSuffix, no, FileName),
> +			{ TimestampFiles = [FileName] }
> +		)
> +	    ).

Hmm, is the "no" there correct?

> +++ compiler/modules.m	2001/07/03 09:59:17
> @@ -577,6 +577,13 @@
>  :- pred touch_interface_datestamp(module_name, string, io__state, io__state).
>  :- mode touch_interface_datestamp(in, in, di, uo) is det.
>  
> +	% touch_file(FileName).
> +	%
> +	% Update the modification time for file `FileName'.
> +	
> +:- pred touch_file(file_name, io__state, io__state).
> +:- mode touch_file(in, di, uo) is det.

The name and documentation of that procedure are misleading;
what the code does is not just to update the modification time,
but also to clobber the file contents.

(For touch_interface_datestamp, that's not an issue, since the
file is a datestamp file, i.e. it is not used for anything except
the datestamp, and its contents are irrelevant.)

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