[m-rev.] for review: Cache transitive foreign imports.

Peter Wang novalazy at gmail.com
Thu Nov 24 14:45:04 AEDT 2022


We previously cached the foreign imports of a module.
find_module_foreign_imports always needs to compute the transitive
foreign imports (i.e. including the foreign imports of transitively
implementation-imported modules), so it had to take the union of a bunch
of sets after looking up cached results.

This change caches the transitive foreign imports of a module, avoiding
the bitset union operations, many/most of which turn out to be redundant.
It reduces the average run time for a do-nothing build of Prince on my
machine from 5.9 s to 3.1 s, for a speed up of ~47%.

compiler/make.dependencies.m:
    Cache transitive foreign module imports instead of direct foreign
    module imports.

    Delete code to make use va_map; it is no longer used.
    (The va_map module may still be convenient to use in places where we
    currently use version_array, so keep it around.)

compiler/make.deps_set.m:
    Delete code to make use of va_map.

compiler/make.make_info.m:
    Rename field.

compiler/make.top_level.m:
    Conform to change.
---
 compiler/make.dependencies.m | 78 +++++++++++++++---------------------
 compiler/make.deps_set.m     |  9 +----
 compiler/make.make_info.m    |  3 +-
 compiler/make.top_level.m    |  2 +-
 4 files changed, 36 insertions(+), 56 deletions(-)

diff --git a/compiler/make.dependencies.m b/compiler/make.dependencies.m
index cd8aa7c7f..200b029ff 100644
--- a/compiler/make.dependencies.m
+++ b/compiler/make.dependencies.m
@@ -151,8 +151,9 @@
 :- type cached_direct_imports.
 :- func init_cached_direct_imports = cached_direct_imports.
 
-:- type cached_foreign_imports.
-:- func init_cached_foreign_imports = cached_foreign_imports.
+:- type cached_transitive_foreign_imports.
+:- func init_cached_transitive_foreign_imports =
+    cached_transitive_foreign_imports.
 
 :- type cached_transitive_dependencies.
 :- func init_cached_transitive_dependencies = cached_transitive_dependencies.
@@ -165,7 +166,6 @@
 :- import_module backend_libs.
 :- import_module backend_libs.compile_target_code.
 :- import_module libs.options.
-:- import_module libs.va_map.
 :- import_module make.module_dep_file.
 :- import_module make.util.
 :- import_module parse_tree.
@@ -709,18 +709,32 @@ foreign_imports(Globals, ModuleIndex, Succeeded, Modules, !Info, !IO) :-
 
 find_module_foreign_imports(Languages, Globals, ModuleIndex, Succeeded,
         ForeignModules, !Info, !IO) :-
-    find_transitive_implementation_imports(Globals, ModuleIndex, Succeeded0,
-        ImportedModules, !Info, !IO),
-    (
-        Succeeded0 = succeeded,
-        deps_set_foldl3_maybe_stop_at_error(!.Info ^ mki_keep_going,
-            union_deps(find_module_foreign_imports_2(Languages)),
-            Globals, insert(ImportedModules, ModuleIndex),
-            Succeeded, init, ForeignModules, !Info, !IO)
-    ;
-        Succeeded0 = did_not_succeed,
-        Succeeded = did_not_succeed,
-        ForeignModules = init
+    % Languages should be constant for the duration of the process,
+    % so is unnecessary to include in the cache key.
+    CachedForeignImports0 = !.Info ^ mki_cached_transitive_foreign_imports,
+    ( if map.search(CachedForeignImports0, ModuleIndex, CachedResult) then
+        CachedResult = deps_result(Succeeded, ForeignModules)
+    else
+        find_transitive_implementation_imports(Globals, ModuleIndex,
+            Succeeded0, ImportedModules, !Info, !IO),
+        (
+            Succeeded0 = succeeded,
+            deps_set_foldl3_maybe_stop_at_error(!.Info ^ mki_keep_going,
+                union_deps(find_module_foreign_imports_2(Languages)),
+                Globals, insert(ImportedModules, ModuleIndex),
+                Succeeded, init, ForeignModules, !Info, !IO),
+            Result = deps_result(Succeeded, ForeignModules),
+            CachedForeignImports1 =
+                !.Info ^ mki_cached_transitive_foreign_imports,
+            map.det_insert(ModuleIndex, Result,
+                CachedForeignImports1, CachedForeignImports),
+            !Info ^ mki_cached_transitive_foreign_imports :=
+                CachedForeignImports
+        ;
+            Succeeded0 = did_not_succeed,
+            Succeeded = did_not_succeed,
+            ForeignModules = init
+        )
     ).
 
 :- pred find_module_foreign_imports_2(set(foreign_language)::in,
@@ -730,30 +744,6 @@ find_module_foreign_imports(Languages, Globals, ModuleIndex, Succeeded,
 
 find_module_foreign_imports_2(Languages, Globals, ModuleIndex, Succeeded,
         ForeignModules, !Info, !IO) :-
-    % Languages should be constant for the duration of the process.
-    CachedForeignImports0 = !.Info ^ mki_cached_foreign_imports,
-    va_map.lookup(CachedForeignImports0, ModuleIndex, MaybeResult0),
-    (
-        MaybeResult0 = yes(Result0),
-        Result0 = deps_result(Succeeded, ForeignModules)
-    ;
-        MaybeResult0 = no,
-        find_module_foreign_imports_3(Languages, Globals, ModuleIndex,
-            Succeeded, ForeignModules, !Info, !IO),
-        Result = deps_result(Succeeded, ForeignModules),
-        CachedForeignImports1 = !.Info ^ mki_cached_foreign_imports,
-        va_map.set(ModuleIndex, yes(Result),
-            CachedForeignImports1, CachedForeignImports),
-        !Info ^ mki_cached_foreign_imports := CachedForeignImports
-    ).
-
-:- pred find_module_foreign_imports_3(set(foreign_language)::in,
-    globals::in, module_index::in,
-    maybe_succeeded::out, deps_set(module_index)::out,
-    make_info::in, make_info::out, io::di, io::uo) is det.
-
-find_module_foreign_imports_3(Languages, Globals, ModuleIndex,
-        Succeeded, ForeignModules, !Info, !IO) :-
     module_index_to_name(!.Info, ModuleIndex, ModuleName),
     get_module_dependencies(Globals, ModuleName, MaybeModuleDepInfo,
         !Info, !IO),
@@ -1380,14 +1370,10 @@ make_write_dependency_file_and_timestamp_list([Head | Tail], !IO) :-
 
 init_cached_direct_imports = map.init.
 
-:- type cached_foreign_imports ==
-    va_map(module_index, maybe(module_deps_result)).
+:- type cached_transitive_foreign_imports
+    == map(module_index, module_deps_result).
 
-:- instance va_map_value(maybe(T)) where [
-    default_value = no
-].
-
-init_cached_foreign_imports = va_map.init.
+init_cached_transitive_foreign_imports = map.init.
 
 :- type transitive_dependencies_root
     --->    transitive_dependencies_root(
diff --git a/compiler/make.deps_set.m b/compiler/make.deps_set.m
index cc890d51f..87df6e0f3 100644
--- a/compiler/make.deps_set.m
+++ b/compiler/make.deps_set.m
@@ -23,8 +23,6 @@
 :- module make.deps_set.
 :- interface.
 
-:- import_module libs.
-:- import_module libs.va_map.
 :- import_module make.dependencies.
 :- import_module make.make_info.
 :- import_module mdbcomp.
@@ -44,7 +42,6 @@
 
 :- type module_index.
 :- instance enum(module_index).
-:- instance va_map_key(module_index).
 
 :- type dependency_file_index.
 :- instance enum(dependency_file_index).
@@ -95,6 +92,7 @@
 
 :- implementation.
 
+:- import_module libs.
 :- import_module parse_tree.
 
 :- import_module int.
@@ -116,11 +114,6 @@
     from_int(I) = module_index(I)
 ].
 
-:- instance va_map_key(module_index) where [
-    ( from_key(module_index(I)) = uint.cast_from_int(I) ),
-    ( to_key(U) = module_index(uint.cast_to_int(U)) )
-].
-
 :- type dependency_file_index
     --->    dependency_file_index(int).
 
diff --git a/compiler/make.make_info.m b/compiler/make.make_info.m
index c6baa60ea..433e8163e 100644
--- a/compiler/make.make_info.m
+++ b/compiler/make.make_info.m
@@ -96,7 +96,8 @@
                 mki_cached_transitive_dependencies
                                         :: cached_transitive_dependencies,
 
-                mki_cached_foreign_imports :: cached_foreign_imports,
+                mki_cached_transitive_foreign_imports
+                                        :: cached_transitive_foreign_imports,
 
                 % Should the `.module_dep' files be rebuilt?
                 % Set to `do_not_rebuild_module_deps' for `mmc --make clean'.
diff --git a/compiler/make.top_level.m b/compiler/make.top_level.m
index 0c959c3fe..8dcdc77a5 100644
--- a/compiler/make.top_level.m
+++ b/compiler/make.top_level.m
@@ -132,7 +132,7 @@ make_process_compiler_args(Globals, DetectedGradeFlags, Variables, OptionArgs,
             init_cached_direct_imports,
             init_cached_direct_imports,
             init_cached_transitive_dependencies,
-            init_cached_foreign_imports,
+            init_cached_transitive_foreign_imports,
             ShouldRebuildModuleDeps,
             KeepGoing,
             ErrorFileModules,
-- 
2.38.0



More information about the reviews mailing list