[m-rev.] for review: Optimise dependency file index maps.

Peter Wang novalazy at gmail.com
Mon Nov 28 15:06:48 AEDT 2022


Looking for a better name than dependency_file_with_module_index.

---

Replace the type used in the mappings between dependency_file and
dependency_file_index from dependency_file, which refers to targets by
module_name, to a similar type that refers to targets by module_index.

make.dependencies.of_3 is called frequently to convert a module_index
and module_target_type to a dependency_file_index, but it previously had
to look up the module_name corresponding to a module_index instead of
using the module_index as given. Furthermore, using module_index as part
of a hash table key should be more efficient than module_name.

This change improves the run time of a do-nothing build of Prince
on my machine is improved from 1.8 s to 1.7 s.

compiler/make.dependencies.m:
    Add the new type.

    Don't need to call module_index_to_name in of_3.

compiler/make.make_info.m:
    Replace dependency_file in the dependency file <-> index maps
    with the new type.

compiler/make.util.m:
    Add hash predicate for new type.

compiler/make.deps_set.m:
compiler/make.top_level.m:
    Conform to changes.

diff --git a/compiler/make.dependencies.m b/compiler/make.dependencies.m
index 23210db09..df6802215 100644
--- a/compiler/make.dependencies.m
+++ b/compiler/make.dependencies.m
@@ -61,6 +61,13 @@
     ;       dep_file(file_name).
             % An ordinary file which `mmc --make' does not know how to rebuild.
 
+    % Like dependency_file but refers to a module by index instead of by name,
+    % which is more efficient when the name is not required.
+    %
+:- type dependency_file_with_module_index
+    --->    dfmi_target(module_index, module_target_type)
+    ;       dfmi_file(file_name).
+
     % Return a closure which will find the dependencies for a target type
     % given a module name.
     %
@@ -401,9 +408,7 @@ of_2(FileType, FindDeps, Globals, ModuleIndex, Succeeded, TargetFiles,
     make_info::in, make_info::out) is det.
 
 of_3(FileType, ModuleIndex, !List, !Info) :-
-    % Could we avoid looking up ModuleName?
-    module_index_to_name(!.Info, ModuleIndex, ModuleName),
-    TargetFile = dep_target(target_file(ModuleName, FileType)),
+    TargetFile = dfmi_target(ModuleIndex, FileType),
     dependency_file_to_index(TargetFile, TargetFileIndex, !Info),
     !:List = [TargetFileIndex | !.List].
 
diff --git a/compiler/make.deps_set.m b/compiler/make.deps_set.m
index 269a4bcc2..e37eb2aec 100644
--- a/compiler/make.deps_set.m
+++ b/compiler/make.deps_set.m
@@ -73,7 +73,7 @@
 
     % Convert a dependency file to a dependency_file_index.
     %
-:- pred dependency_file_to_index(dependency_file::in,
+:- pred dependency_file_to_index(dependency_file_with_module_index::in,
     dependency_file_index::out, make_info::in, make_info::out) is det.
 
     % Convert a list of dependency files to a dependency_file_index set.
@@ -217,8 +217,16 @@ dependency_files_to_index_set(DepFiles, DepIndexSet, !Info) :-
     deps_set(dependency_file_index)::in, deps_set(dependency_file_index)::out,
     make_info::in, make_info::out) is det.
 
-dependency_files_to_index_set_2(DepFiles, !Set, !Info) :-
-    dependency_file_to_index(DepFiles, DepIndex, !Info),
+dependency_files_to_index_set_2(DepFile0, !Set, !Info) :-
+    (
+        DepFile0 = dep_target(target_file(ModuleName, TargetType)),
+        module_name_to_index(ModuleName, ModuleIndex, !Info),
+        DepFile = dfmi_target(ModuleIndex, TargetType)
+    ;
+        DepFile0 = dep_file(FileName),
+        DepFile = dfmi_file(FileName)
+    ),
+    dependency_file_to_index(DepFile, DepIndex, !Info),
     insert(DepIndex, !Set).
 
 %---------------------------------------------------------------------------%
@@ -230,7 +238,15 @@ index_to_dependency_file(Info, Index, DepFile) :-
     Info ^ mki_dep_file_index_map =
         dependency_file_index_map(_Forward, Reverse, _Size),
     Index = dependency_file_index(I),
-    DepFile = version_array.lookup(Reverse, I).
+    DepFile0 = version_array.lookup(Reverse, I),
+    (
+        DepFile0 = dfmi_target(ModuleIndex, FileType),
+        module_index_to_name(Info, ModuleIndex, ModuleName),
+        DepFile = dep_target(target_file(ModuleName, FileType))
+    ;
+        DepFile0 = dfmi_file(FileName),
+        DepFile = dep_file(FileName)
+    ).
 
 %---------------------%
 
diff --git a/compiler/make.make_info.m b/compiler/make.make_info.m
index 8ae0b1378..c7d0127a8 100644
--- a/compiler/make.make_info.m
+++ b/compiler/make.make_info.m
@@ -161,9 +161,11 @@
 
 :- type dependency_file_index_map
     --->    dependency_file_index_map(
-                dfim_forward_map        :: version_hash_table(dependency_file,
+                dfim_forward_map        :: version_hash_table(
+                                            dependency_file_with_module_index,
                                             dependency_file_index),
-                dfim_reverse_map        :: version_array(dependency_file),
+                dfim_reverse_map        :: version_array(
+                                            dependency_file_with_module_index),
                 dfim_counter            :: int
             ).
 
diff --git a/compiler/make.top_level.m b/compiler/make.top_level.m
index 3f694c09d..f1aac8fda 100644
--- a/compiler/make.top_level.m
+++ b/compiler/make.top_level.m
@@ -95,7 +95,8 @@ make_process_compiler_args(Globals, DetectedGradeFlags, Variables, OptionArgs,
             version_hash_table.init_default(module_name_hash),
             version_array.empty, 0),
         DepIndexMap = dependency_file_index_map(
-            version_hash_table.init_default(dependency_file_hash),
+            version_hash_table.init_default(
+                dependency_file_with_module_index_hash),
             version_array.empty, 0),
         DepStatusMap = version_hash_table.init_default(dependency_file_hash),
 
diff --git a/compiler/make.util.m b/compiler/make.util.m
index c87417f63..f7d4385be 100644
--- a/compiler/make.util.m
+++ b/compiler/make.util.m
@@ -256,6 +256,9 @@
 
 :- pred dependency_file_hash(dependency_file::in, int::out) is det.
 
+:- pred dependency_file_with_module_index_hash(
+    dependency_file_with_module_index::in, int::out) is det.
+
 %---------------------------------------------------------------------------%
 %---------------------------------------------------------------------------%
 
@@ -272,6 +275,7 @@
 
 :- import_module bool.
 :- import_module dir.
+:- import_module enum.
 :- import_module int.
 :- import_module io.file.
 :- import_module map.
@@ -1133,6 +1137,17 @@ dependency_file_hash(DepFile, Hash) :-
         Hash = string.hash(FileName)
     ).
 
+dependency_file_with_module_index_hash(DepFile, Hash) :-
+    (
+        DepFile = dfmi_target(ModuleIndex, Type),
+        Hash0 = to_int(ModuleIndex),
+        Hash1 = module_target_type_to_nonce(Type),
+        Hash = mix(Hash0, Hash1)
+    ;
+        DepFile = dfmi_file(FileName),
+        Hash = string.hash(FileName)
+    ).
+
 :- func target_file_hash(target_file) = int.
 
 target_file_hash(TargetFile) = Hash :-
-- 
2.38.0



More information about the reviews mailing list