[m-rev.] for review: Fix performance regression in mmc --make.

[Peter Wang]

In commit 083d376e6598628362ee91c2da170febd83590f4,
dependency_status/7 was made to call get_make_target_file_name/6
for all dep_targets, even in the common case where the status was
already known and the filename was not required.
Since dependency_status is called very often, this extra work was
noticeable when building larger projects.

A subsequent change, commit 9eb003458bedd7462ad4393b42759d136aca6bf3,
made get_dependency_status/7 (renamed from dependency_status)
also return the filename of the dependency alongside the status.
This was to avoid requiring its callers to compute the filename,
but it also obliged get_dependency_status/7 to compute the filename
even when it wasn't otherwise required.

compiler/make.dependencies.m:
    Partially revert the second change by making get_dependency_status
    return a maybe(file_name) instead of file_name.

    Make get_dependency_status avoid calling get_make_target_file_name
    when possible.

    Add predicates to compute the file_names that get_dependency_status
    declined to compute.

    Conform to those changes.
---
 compiler/make.dependencies.m | 58 +++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/compiler/make.dependencies.m b/compiler/make.dependencies.m
index 913789427..459c1ab4c 100644
--- a/compiler/make.dependencies.m
+++ b/compiler/make.dependencies.m
@@ -93,7 +93,7 @@
 %---------------------------------------------------------------------------%
 
 :- pred get_dependency_status(globals::in, dependency_file::in,
-    {dependency_file, file_name, dependency_status}::out,
+    {dependency_file, maybe(file_name), dependency_status}::out,
     make_info::in, make_info::out, io::di, io::uo) is det.
 
 %---------------------------------------------------------------------------%
@@ -120,7 +120,7 @@
     %
 :- pred check_dependency_timestamps(globals::in, file_name::in,
     maybe_error(timestamp)::in, maybe_succeeded::in,
-    list({dependency_file, file_name, dependency_status})::in,
+    list({dependency_file, maybe(file_name), dependency_status})::in,
     list(maybe_error(timestamp))::in, dependencies_result::out,
     io::di, io::uo) is det.
 
@@ -1366,6 +1366,7 @@ make_local_module_id_option(ModuleName, Opts0, Opts) :-
 get_dependency_status(Globals, Dep, Tuple, !Info, !IO) :-
     (
         Dep = dep_file(TargetFileName),
+        MaybeTargetFileName = yes(TargetFileName),
         DepStatusMap0 = make_info_get_dependency_status(!.Info),
         ( if version_hash_table.search(DepStatusMap0, Dep, StatusPrime) then
             Status = StatusPrime
@@ -1386,7 +1387,6 @@ get_dependency_status(Globals, Dep, Tuple, !Info, !IO) :-
     ;
         Dep = dep_target(Target),
         Target = target_file(ModuleName, FileType),
-        get_make_target_file_name(Globals, $pred, Target, TargetFileName, !IO),
         ( if
             ( FileType = module_target_source
             ; FileType = module_target_track_flags
@@ -1397,6 +1397,9 @@ get_dependency_status(Globals, Dep, Tuple, !Info, !IO) :-
             % so are also up-to-date.
             ModuleTarget = module_target(module_target_source),
             TopTargetFile = top_target_file(ModuleName, ModuleTarget),
+            get_make_target_file_name(Globals, $pred, Target,
+                TargetFileName, !IO),
+            MaybeTargetFileName = yes(TargetFileName),
             maybe_warn_up_to_date_target_msg(Globals, TopTargetFile,
                 TargetFileName, !Info, UpToDateMsg),
             % XXX MAKE_STREAM
@@ -1406,8 +1409,14 @@ get_dependency_status(Globals, Dep, Tuple, !Info, !IO) :-
             DepStatusMap0 = make_info_get_dependency_status(!.Info),
             version_hash_table.search(DepStatusMap0, Dep, StatusPrime)
         then
-            Status = StatusPrime
+            Status = StatusPrime,
+            % Avoid calling get_make_target_file_name in this common case --
+            % it makes a big difference to performance.
+            MaybeTargetFileName = no
         else
+            get_make_target_file_name(Globals, $pred, Target, TargetFileName,
+                !IO),
+            MaybeTargetFileName = yes(TargetFileName),
             get_module_dependencies(Globals, ModuleName, MaybeModuleDepInfo,
                 !Info, !IO),
             (
@@ -1444,8 +1453,35 @@ get_dependency_status(Globals, Dep, Tuple, !Info, !IO) :-
             make_info_set_dependency_status(DepStatusMap, !Info)
         )
     ),
+    Tuple = {Dep, MaybeTargetFileName, Status}.
+
+%---------------------------------------------------------------------------%
+
+:- pred get_dependency_file_names(globals::in,
+    list({dependency_file, maybe(file_name), dependency_status})::in,
+    list({dependency_file, file_name, dependency_status})::out,
+    io::di, io::uo) is det.
+
+get_dependency_file_names(Globals, Tuples0, Tuples, !IO) :-
+    list.map_foldl(get_dependency_file_name(Globals), Tuples0, Tuples, !IO).
+
+:- pred get_dependency_file_name(globals::in,
+    {dependency_file, maybe(file_name), dependency_status}::in,
+    {dependency_file, file_name, dependency_status}::out,
+    io::di, io::uo) is det.
+
+get_dependency_file_name(Globals, Tuple0, Tuple, !IO) :-
+    Tuple0 = {Dep, MaybeTargetFileName, Status},
+    (
+        MaybeTargetFileName = yes(TargetFileName)
+    ;
+        MaybeTargetFileName = no,
+        dependency_file_to_file_name(Globals, Dep, TargetFileName, !IO)
+    ),
     Tuple = {Dep, TargetFileName, Status}.
 
+%---------------------------------------------------------------------------%
+
 check_dependencies(Globals, TargetFileName, MaybeTimestamp, BuildDepsSucceeded,
         DepFiles, DepsResult, !Info, !IO) :-
     list.map_foldl2(get_dependency_status(Globals), DepFiles, DepStatusTuples,
@@ -1453,9 +1489,11 @@ check_dependencies(Globals, TargetFileName, MaybeTimestamp, BuildDepsSucceeded,
     list.filter(
         ( pred(({_, _, DepStatus})::in) is semidet :-
             DepStatus \= deps_status_up_to_date
-        ), DepStatusTuples, UnbuiltDependencyTuples),
+        ), DepStatusTuples, UnbuiltDependencyTuples0),
     (
-        UnbuiltDependencyTuples = [_ | _],
+        UnbuiltDependencyTuples0 = [_ | _],
+        get_dependency_file_names(Globals,
+            UnbuiltDependencyTuples0, UnbuiltDependencyTuples, !IO),
         debug_make_msg(Globals,
             describe_unbuilt_dependencies(TargetFileName,
                 UnbuiltDependencyTuples),
@@ -1464,7 +1502,7 @@ check_dependencies(Globals, TargetFileName, MaybeTimestamp, BuildDepsSucceeded,
         maybe_write_msg(DebugMsg, !IO),
         DepsResult = deps_error
     ;
-        UnbuiltDependencyTuples = [],
+        UnbuiltDependencyTuples0 = [],
         debug_make_msg(Globals,
             string.format("%s: finished dependencies\n",
                 [s(TargetFileName)]),
@@ -1543,7 +1581,7 @@ check_dependencies_timestamps_missing_deps_msg(TargetFileName,
     ).
 
 check_dependency_timestamps(Globals, TargetFileName, MaybeTimestamp,
-        BuildDepsSucceeded, DepFileTuples, DepTimestamps, DepsResult, !IO) :-
+        BuildDepsSucceeded, DepFileTuples0, DepTimestamps, DepsResult, !IO) :-
     (
         MaybeTimestamp = error(_),
         DepsResult = deps_out_of_date,
@@ -1556,6 +1594,8 @@ check_dependency_timestamps(Globals, TargetFileName, MaybeTimestamp,
         MaybeTimestamp = ok(Timestamp),
         ( if error_in_timestamps(DepTimestamps) then
             DepsResult = deps_error,
+            get_dependency_file_names(Globals, DepFileTuples0, DepFileTuples,
+                !IO),
             (
                 BuildDepsSucceeded = succeeded,
                 % Something has gone wrong -- building the target has
@@ -1587,6 +1627,8 @@ check_dependency_timestamps(Globals, TargetFileName, MaybeTimestamp,
             ;
                 Rebuild = no,
                 ( if newer_timestamp(DepTimestamps, Timestamp) then
+                    get_dependency_file_names(Globals,
+                        DepFileTuples0, DepFileTuples, !IO),
                     debug_make_msg(Globals,
                         describe_newer_dependencies(TargetFileName,
                             MaybeTimestamp, DepFileTuples, DepTimestamps),
-- 
2.39.0