[m-rev.] for review: change mmc --make handling of invalid module analysis
Peter Wang
novalazy at gmail.com
Mon Aug 4 17:57:42 AEST 2008
Branches: main
Try to improve `mmc --make' behaviour when performing intermodule analysis,
with regards to module .analysis files which become invalid.
When analysis of one module causes other modules to become invalid, go back
and immediately reanalyse all invalid modules. Previously we would only do
that after making a complete analysis pass over the suboptimal modules. So
this change tries to keep the number of invalid modules to a minimum.
The rationale is that a module B may depend on answers for a now invalid
module A. Analysing B while A is invalid means B can't safely use any
answers for A. On the other hand, chances are that the specific answer B
depends on wasn't actually invalid; module A was just invalidated because
some *other* answer was invalidated. By analysing module A again first,
module B may be able to make use of that answer by the time we get to
analysing it.
compiler/make.program_target.m:
Implement the above.
compiler/make.util.m:
Split "Reanalysing invalid/suboptimal modules" messages into two
messages to give the user a better idea of what is happening.
diff --git a/compiler/make.program_target.m b/compiler/make.program_target.m
index 029c86a..e7e1df7 100644
--- a/compiler/make.program_target.m
+++ b/compiler/make.program_target.m
@@ -841,6 +841,22 @@ remove_cache_dir(CacheDir, !Info, !IO) :-
%-----------------------------------------------------------------------------%
+% The way we analyse modules is as follows:
+%
+% - always analyse invalid modules before reanalysing suboptimal modules;
+% - try to analyse modules at the bottom of the dependency graph first;
+% - after each suboptimal module is reanalysed, check if any modules were
+% invalidated. If so, "restart" the analysis procedure by analysing invalid
+% modules first. Don't reanalyse suboptimal modules again until there are no
+% invalid modules.
+%
+% Sometimes the module dependency graph may contain cycles so that in large
+% clumps we can't tell the "bottom" modules from "top" modules. However, it
+% might be possible to improve the order in which we analyse modules by
+% extracting some metric from IMDG files. e.g. it could be better to analyse
+% modules which have more lines in their IMDG files first. This is not yet
+% implemented.
+
:- pred build_analysis_files(module_name::in, list(module_name)::in,
bool::in, bool::out, make_info::in, make_info::out, io::di, io::uo)
is det.
@@ -865,15 +881,15 @@ build_analysis_files(MainModuleName, AllModules, Succeeded0, Succeeded,
->
Succeeded = no
;
- build_analysis_files_1(MainModuleName, AllModules,
+ build_analysis_files_2(MainModuleName, AllModules,
Succeeded, !Info, !IO)
)
).
-:- pred build_analysis_files_1(module_name::in, list(module_name)::in,
+:- pred build_analysis_files_2(module_name::in, list(module_name)::in,
bool::out, make_info::in, make_info::out, io::di, io::uo) is det.
-build_analysis_files_1(MainModuleName, AllModules, Succeeded, !Info, !IO) :-
+build_analysis_files_2(MainModuleName, AllModules, Succeeded, !Info, !IO) :-
get_target_modules(module_target_analysis_registry, AllModules,
TargetModules0, !Info, !IO),
reverse_ordered_modules(!.Info ^ module_dependencies,
@@ -885,50 +901,121 @@ build_analysis_files_1(MainModuleName, AllModules, Succeeded, !Info, !IO) :-
LocalModulesOpts, !Info, !IO),
(
Succeeded0 = yes,
- build_analysis_files_2(MainModuleName, TargetModules,
- LocalModulesOpts, Succeeded0, Succeeded, !Info, !IO)
+ build_invalid_analysis_files_step(MainModuleName, TargetModules,
+ LocalModulesOpts, yes, Succeeded, !Info, !IO)
;
Succeeded0 = no,
Succeeded = no
).
-:- pred build_analysis_files_2(module_name::in, list(module_name)::in,
- list(string)::in, bool::in, bool::out, make_info::in, make_info::out,
- io::di, io::uo) is det.
+:- pred build_invalid_analysis_files_step(module_name::in,
+ list(module_name)::in, list(string)::in, bool::in, bool::out,
+ make_info::in, make_info::out, io::di, io::uo) is det.
-build_analysis_files_2(MainModuleName, TargetModules, LocalModulesOpts,
- Succeeded0, Succeeded, !Info, !IO) :-
+build_invalid_analysis_files_step(MainModuleName, TargetModules,
+ LocalModulesOpts, FirstCall, Succeeded, !Info, !IO) :-
+ divide_modules_by_analysis_status(yes, TargetModules, ValidModules,
+ InvalidModules, !IO),
globals.io_lookup_bool_option(keep_going, KeepGoing, !IO),
- foldl2_maybe_stop_at_error(KeepGoing,
- make_module_target_extra_options(LocalModulesOpts),
- make_dependency_list(TargetModules, module_target_analysis_registry),
- Succeeded1, !Info, !IO),
- % Maybe we should have an option to reanalyse cliques before moving
- % upwards in the dependency graph?
-
- % Find which module analysis files are suboptimal or invalid.
- % If there are any invalid files then we repeat the analysis pass.
- % If there are only suboptimal files then we repeat the analysis up
- % to the number of times given by the user.
- ReanalysisPasses = !.Info ^ reanalysis_passes,
- ReanalyseSuboptimal = (if ReanalysisPasses > 1 then yes else no),
- modules_needing_reanalysis(ReanalyseSuboptimal, TargetModules,
- InvalidModules, SuboptimalModules, !IO),
- ( list.is_not_empty(InvalidModules) ->
- maybe_reanalyse_modules_message(!IO),
- list.foldl(reset_analysis_registry_dependency_status,
- InvalidModules, !Info),
- build_analysis_files_2(MainModuleName, TargetModules, LocalModulesOpts,
- Succeeded0, Succeeded, !Info, !IO)
- ; list.is_not_empty(SuboptimalModules) ->
- list.foldl(reset_analysis_registry_dependency_status,
- SuboptimalModules, !Info),
- !:Info = !.Info ^ reanalysis_passes := ReanalysisPasses - 1,
- maybe_reanalyse_modules_message(!IO),
- build_analysis_files_2(MainModuleName, TargetModules, LocalModulesOpts,
- Succeeded0, Succeeded, !Info, !IO)
+ (
+ InvalidModules = [_ | _],
+ foldl2_maybe_stop_at_error(KeepGoing,
+ make_module_target_extra_options(LocalModulesOpts),
+ make_dependency_list(InvalidModules,
+ module_target_analysis_registry),
+ Succeeded1, !Info, !IO),
+ (
+ Succeeded1 = yes,
+ % Repeat until there are no invalid modules.
+ list.foldl(reset_analysis_registry_dependency_status,
+ TargetModules, !Info),
+ build_invalid_analysis_files_step(MainModuleName, TargetModules,
+ LocalModulesOpts, no, Succeeded, !Info, !IO)
+ ;
+ Succeeded1 = no,
+ Succeeded = no
+ )
;
- Succeeded = Succeeded0 `and` Succeeded1
+ InvalidModules = [],
+ (
+ ValidModules = [_ | _],
+ (
+ FirstCall = yes
+ ;
+ FirstCall = no,
+ maybe_analyse_valid_modules_message(!IO)
+ ),
+ build_valid_analysis_files_step(MainModuleName, TargetModules,
+ LocalModulesOpts, ValidModules, Succeeded, !Info, !IO)
+ ;
+ ValidModules = [],
+ Succeeded = yes
+ )
+ ).
+
+:- pred build_valid_analysis_files_step(module_name::in, list(module_name)::in,
+ list(string)::in, list(module_name)::in, bool::out,
+ make_info::in, make_info::out, io::di, io::uo) is det.
+
+build_valid_analysis_files_step(MainModuleName, TargetModules,
+ LocalModulesOpts, Modules, Succeeded, !Info, !IO) :-
+ (
+ Modules = [],
+ % Repeat analysis if there are suboptimal modules, as many times as
+ % desired.
+ Passes = !.Info ^ reanalysis_passes,
+ ( Passes > 0 ->
+ !Info ^ reanalysis_passes := Passes - 1,
+ divide_modules_by_analysis_status(no, TargetModules,
+ SuboptimalModules, InvalidModules, !IO),
+ (
+ InvalidModules = []
+ ;
+ InvalidModules = [_ | _],
+ unexpected(this_file,
+ "build_valid_analysis_files_step: invalid modules")
+ ),
+ (
+ SuboptimalModules = [_ | _],
+ maybe_analyse_valid_modules_message(!IO),
+ list.foldl(reset_analysis_registry_dependency_status,
+ TargetModules, !Info),
+ build_valid_analysis_files_step(MainModuleName, TargetModules,
+ LocalModulesOpts, SuboptimalModules, Succeeded, !Info, !IO)
+ ;
+ SuboptimalModules = [],
+ Succeeded = yes
+ )
+ ;
+ Succeeded = yes
+ )
+ ;
+ Modules = [FirstModule | RestModules],
+ make_module_target_extra_options(LocalModulesOpts,
+ dep_target(target_file(FirstModule,
+ module_target_analysis_registry)),
+ Succeeded0, !Info, !IO),
+ (
+ Succeeded0 = yes,
+ % If analysis of this module caused some other modules to become
+ % invalid, immediately go back to analysing invalid modules.
+ all_valid_modules(TargetModules, AllValid, !IO),
+ (
+ AllValid = yes,
+ build_valid_analysis_files_step(MainModuleName, TargetModules,
+ LocalModulesOpts, RestModules, Succeeded, !Info, !IO)
+ ;
+ AllValid = no,
+ list.foldl(reset_analysis_registry_dependency_status,
+ TargetModules, !Info),
+ maybe_analyse_invalid_modules_message(!IO),
+ build_invalid_analysis_files_step(MainModuleName,
+ TargetModules, LocalModulesOpts, no, Succeeded, !Info, !IO)
+ )
+ ;
+ Succeeded0 = no,
+ Succeeded = no
+ )
).
% Return a list of modules in reverse order of their dependencies, i.e.
@@ -959,35 +1046,51 @@ lookup_module_imports(ModuleDeps, ModuleName) = ModuleImports :-
unexpected(this_file, "lookup_module_imports")
).
-:- pred modules_needing_reanalysis(bool::in, list(module_name)::in,
+:- pred divide_modules_by_analysis_status(bool::in, list(module_name)::in,
list(module_name)::out, list(module_name)::out, io::di, io::uo) is det.
-modules_needing_reanalysis(_, [], [], [], !IO).
-modules_needing_reanalysis(ReanalyseSuboptimal, [Module | Modules],
- InvalidModules, SuboptimalModules, !IO) :-
+divide_modules_by_analysis_status(_IncludeOptimal, [], [], [], !IO).
+divide_modules_by_analysis_status(IncludeOptimal, [Module | Modules],
+ ValidModules, InvalidModules, !IO) :-
read_module_overall_status(mmc, Module, ModuleStatus, !IO),
(
ModuleStatus = optimal,
- modules_needing_reanalysis(ReanalyseSuboptimal, Modules,
- InvalidModules, SuboptimalModules, !IO)
+ IncludeOptimal = no,
+ divide_modules_by_analysis_status(IncludeOptimal, Modules,
+ ValidModules, InvalidModules, !IO)
;
- ModuleStatus = suboptimal,
- modules_needing_reanalysis(ReanalyseSuboptimal, Modules,
- InvalidModules, SuboptimalModules0, !IO),
(
- ReanalyseSuboptimal = yes,
- SuboptimalModules = [Module | SuboptimalModules0]
+ ModuleStatus = suboptimal
;
- ReanalyseSuboptimal = no,
- SuboptimalModules = SuboptimalModules0
- )
+ ModuleStatus = optimal,
+ IncludeOptimal = yes
+ ),
+ divide_modules_by_analysis_status(IncludeOptimal, Modules,
+ ValidModules0, InvalidModules, !IO),
+ ValidModules = [Module | ValidModules0]
;
ModuleStatus = invalid,
- modules_needing_reanalysis(ReanalyseSuboptimal, Modules,
- InvalidModules0, SuboptimalModules, !IO),
+ divide_modules_by_analysis_status(IncludeOptimal, Modules,
+ ValidModules, InvalidModules0, !IO),
InvalidModules = [Module | InvalidModules0]
).
+:- pred all_valid_modules(list(module_name)::in, bool::out, io::di, io::uo)
+ is det.
+
+all_valid_modules([], yes, !IO).
+all_valid_modules([Module | Modules], AllValid, !IO) :-
+ read_module_overall_status(mmc, Module, ModuleStatus, !IO),
+ (
+ ( ModuleStatus = optimal
+ ; ModuleStatus = suboptimal
+ ),
+ all_valid_modules(Modules, AllValid, !IO)
+ ;
+ ModuleStatus = invalid,
+ AllValid = no
+ ).
+
:- pred reset_analysis_registry_dependency_status(module_name::in,
make_info::in, make_info::out) is det.
diff --git a/compiler/make.util.m b/compiler/make.util.m
index 25e0426..de9d4d1 100644
--- a/compiler/make.util.m
+++ b/compiler/make.util.m
@@ -270,10 +270,14 @@
:- pred maybe_make_target_message_to_stream(io.output_stream::in,
target_file::in, io::di, io::uo) is det.
- % Write a message "Reanalysing invalid/suboptimal modules" if
+ % Write a message "Analysing optimal/suboptimal modules" if
% `--verbose-make' is set.
%
-:- pred maybe_reanalyse_modules_message(io::di, io::uo) is det.
+:- pred maybe_analyse_valid_modules_message(io::di, io::uo) is det.
+
+ % Write a message "Analysing invalid modules" if `--verbose-make' is set.
+ %
+:- pred maybe_analyse_invalid_modules_message(io::di, io::uo) is det.
% Write a message "** Error making <filename>".
%
@@ -1527,12 +1531,21 @@ maybe_make_target_message_to_stream(OutputStream, TargetFile, !IO) :-
io.set_output_stream(OldOutputStream, _, !IO)
), !IO).
-maybe_reanalyse_modules_message(!IO) :-
+maybe_analyse_valid_modules_message(!IO) :-
+ io.output_stream(OutputStream, !IO),
+ verbose_msg(
+ (pred(!.IO::di, !:IO::uo) is det :-
+ io.set_output_stream(OutputStream, OldOutputStream, !IO),
+ io.write_string("Analysing valid modules\n", !IO),
+ io.set_output_stream(OldOutputStream, _, !IO)
+ ), !IO).
+
+maybe_analyse_invalid_modules_message(!IO) :-
io.output_stream(OutputStream, !IO),
verbose_msg(
(pred(!.IO::di, !:IO::uo) is det :-
io.set_output_stream(OutputStream, OldOutputStream, !IO),
- io.write_string("Reanalysing invalid/suboptimal modules\n", !IO),
+ io.write_string("Analysing invalid modules\n", !IO),
io.set_output_stream(OldOutputStream, _, !IO)
), !IO).
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to: mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions: mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------
More information about the reviews
mailing list