[m-rev.] for review: Cache timestamps by target file.
Peter Wang
novalazy at gmail.com
Wed Nov 30 12:04:54 AEDT 2022
Previously, to look up the timestamp for a target_file we would first
compute a file name for that target_file, then look up the timestamp by
file name. This occurs frequently, so even though the computed file name
was cached, eliminating one of the two steps can be worthwhile.
This change therefore introduces a cache of timestamps indexed by
target_file, in addition to the existing cache of timestamps indexed
by file name.
Cache invalidation can be complicated because when a file is updated,
it is not straightforward to which target_file(s) may be affected.
Invalidating the entire cache is viable as it happens relatively
infrequently, and the cache will be quickly repopulated anyway.
This change improves the run time of a do-nothing build of Prince
on my machine from 1.7 s to 1.6 s.
compiler/make.make_info.m:
compiler/make.top_level.m:
Add a field to make_info to hold a cache of timestamps indexed by
target_file.
Delete the mki_search_file_name_cache field.
compiler/make.util.m:
Cache timestamps by target_file in get_target_timestamp.
Avoid computing file names until required.
Don't cache computed file names in get_file_name.
get_file_name is now called much less frequently, so caching the
result should not be necessary (but can be reintroduced if some
other code path requires it).
Clear out timestamps for target_files when a file is deleted.
compiler/make.module_target.m:
Delete old timestamp for a target_file when the target is made.
compiler/make.program_target.m:
Clear out timestamps for target_files when Java class files are
made.
---
compiler/make.make_info.m | 10 +++----
compiler/make.module_target.m | 19 +++++++-----
compiler/make.program_target.m | 19 ++++++++----
compiler/make.top_level.m | 4 +--
compiler/make.util.m | 54 +++++++++++++++++++---------------
5 files changed, 62 insertions(+), 44 deletions(-)
diff --git a/compiler/make.make_info.m b/compiler/make.make_info.m
index c7d0127a8..d78e6e1df 100644
--- a/compiler/make.make_info.m
+++ b/compiler/make.make_info.m
@@ -2,7 +2,7 @@
% vim: ft=mercury ts=4 sw=4 et
%---------------------------------------------------------------------------%
% Copyright (C) 2002-2012 The University of Melbourne.
-% Copyright (C) 2013-2021 The Mercury team.
+% Copyright (C) 2013-2022 The Mercury team.
% This file may only be copied under the terms of the GNU General
% Public License - see the file COPYING in the Mercury distribution.
%---------------------------------------------------------------------------%
@@ -28,7 +28,6 @@
:- import_module mdbcomp.
:- import_module mdbcomp.sym_name.
:- import_module parse_tree.
-:- import_module parse_tree.file_names.
:- import_module parse_tree.module_dep_info.
:- import_module parse_tree.read_modules.
@@ -59,8 +58,8 @@
mki_file_timestamps :: file_timestamps,
- % Cache chosen file names for a module name and extension.
- mki_search_file_name_cache :: map(module_name_ext, file_name),
+ mki_target_file_timestamps
+ :: target_file_timestamps,
% Any flags required to set detected library grades.
mki_detected_grade_flags :: list(string),
@@ -148,8 +147,7 @@
:- type file_timestamps == map(string, maybe_error(timestamp)).
-:- type module_name_ext
- ---> module_name_ext(module_name, ext).
+:- type target_file_timestamps == map(target_file, timestamp).
:- type module_index_map
---> module_index_map(
diff --git a/compiler/make.module_target.m b/compiler/make.module_target.m
index 160e88549..5d1184d2d 100644
--- a/compiler/make.module_target.m
+++ b/compiler/make.module_target.m
@@ -810,11 +810,12 @@ record_made_target_given_maybe_touched_files(Globals, Succeeded, TargetFile,
list.map_foldl2(get_file_name(Globals, do_not_search), TouchedTargetFiles,
TouchedTargetFileNames, !Info, !IO),
- some [!Timestamps] (
- !:Timestamps = !.Info ^ mki_file_timestamps,
+ some [!FileTimestamps] (
+ !:FileTimestamps = !.Info ^ mki_file_timestamps,
list.foldl(delete_timestamp(Globals), TouchedTargetFileNames,
- !Timestamps),
- list.foldl(delete_timestamp(Globals), OtherTouchedFiles, !Timestamps),
+ !FileTimestamps),
+ list.foldl(delete_timestamp(Globals), OtherTouchedFiles,
+ !FileTimestamps),
% When an .analysis file is made, that potentially invalidates other
% .analysis files so we have to delete their timestamps. The exact list
@@ -823,13 +824,17 @@ record_made_target_given_maybe_touched_files(Globals, Succeeded, TargetFile,
% timestamps of all the .analysis files that we know about.
( if TargetFile = target_file(_, module_target_analysis_registry) then
map.foldl(delete_analysis_registry_timestamps(Globals),
- !.Timestamps, !Timestamps)
+ !.FileTimestamps, !FileTimestamps)
else
true
),
- !Info ^ mki_file_timestamps := !.Timestamps
- ).
+ !Info ^ mki_file_timestamps := !.FileTimestamps
+ ),
+
+ TargetFileTimestamps0 = !.Info ^ mki_target_file_timestamps,
+ map.delete(TargetFile, TargetFileTimestamps0, TargetFileTimestamps),
+ !Info ^ mki_target_file_timestamps := TargetFileTimestamps.
:- pred update_target_status(dependency_status::in, target_file::in,
make_info::in, make_info::out) is det.
diff --git a/compiler/make.program_target.m b/compiler/make.program_target.m
index 987fb9886..cb23d289b 100644
--- a/compiler/make.program_target.m
+++ b/compiler/make.program_target.m
@@ -527,8 +527,12 @@ build_linked_target_2(Globals, MainModuleName, FileType, OutputFileName,
(
InitObjectResult1 = yes(InitObject),
% We may need to update the timestamp of the `_init.o' file.
- !Info ^ mki_file_timestamps :=
- map.delete(!.Info ^ mki_file_timestamps, InitObject),
+ FileTimestamps0 = !.Info ^ mki_file_timestamps,
+ map.delete(InitObject, FileTimestamps0, FileTimestamps1),
+ !Info ^ mki_file_timestamps := FileTimestamps1,
+ % There is no module_target_type for the `_init.o' file,
+ % so mki_target_file_timestamps should not contain anything
+ % that needs to be invalidated.
InitObjects = [InitObject],
DepsResult2 = BuildDepsResult
;
@@ -654,9 +658,12 @@ build_linked_target_2(Globals, MainModuleName, FileType, OutputFileName,
!Info ^ mki_command_line_targets := CmdLineTargets,
(
Succeeded = succeeded,
- FileTimestamps0 = !.Info ^ mki_file_timestamps,
- map.delete(OutputFileName, FileTimestamps0, FileTimestamps),
+ FileTimestamps2 = !.Info ^ mki_file_timestamps,
+ map.delete(OutputFileName, FileTimestamps2, FileTimestamps),
!Info ^ mki_file_timestamps := FileTimestamps
+ % There is no module_target_type for the linked target,
+ % so mki_target_file_timestamps should not contain anything
+ % that needs to be invalidated.
;
Succeeded = did_not_succeed,
file_error(!.Info, OutputFileName, !IO)
@@ -730,7 +737,9 @@ make_java_files(Globals, MainModuleName, ObjModules, Succeeded, !Info, !IO) :-
Timestamps0 = !.Info ^ mki_file_timestamps,
map.foldl(delete_java_class_timestamps, Timestamps0,
map.init, Timestamps),
- !Info ^ mki_file_timestamps := Timestamps
+ !Info ^ mki_file_timestamps := Timestamps,
+ % For simplicity, clear out all target file timestamps.
+ !Info ^ mki_target_file_timestamps := map.init
).
:- pred out_of_date_java_modules(globals::in, list(module_name)::in,
diff --git a/compiler/make.top_level.m b/compiler/make.top_level.m
index f1aac8fda..4dda14099 100644
--- a/compiler/make.top_level.m
+++ b/compiler/make.top_level.m
@@ -116,14 +116,14 @@ make_process_compiler_args(Globals, DetectedGradeFlags, Variables, OptionArgs,
map.init(ModuleDependencies),
map.init(FileTimestamps),
- map.init(SearchFileNameCache),
+ map.init(TargetTimestamps),
set.init(ErrorFileModules),
MaybeImportingModule = maybe.no,
MaybeStdoutLock = maybe.no,
MakeInfo0 = make_info(
ModuleDependencies,
FileTimestamps,
- SearchFileNameCache,
+ TargetTimestamps,
DetectedGradeFlags,
OptionArgs,
Variables,
diff --git a/compiler/make.util.m b/compiler/make.util.m
index f7d4385be..7e47d8954 100644
--- a/compiler/make.util.m
+++ b/compiler/make.util.m
@@ -330,13 +330,34 @@ get_dependency_timestamp(Globals, DependencyFile, MaybeTimestamp, !Info,
get_target_timestamp(Globals, Search, TargetFile, MaybeTimestamp, !Info,
!IO) :-
TargetFile = target_file(_ModuleName, TargetType),
- get_file_name(Globals, Search, TargetFile, FileName, !Info, !IO),
( if TargetType = module_target_analysis_registry then
+ get_file_name(Globals, Search, TargetFile, FileName, !Info, !IO),
get_target_timestamp_analysis_registry(Globals, Search, TargetFile,
FileName, MaybeTimestamp, !Info, !IO)
else
- get_target_timestamp_2(Globals, Search, TargetFile,
- FileName, MaybeTimestamp, !Info, !IO)
+ % This path is hit very frequently so it is worth caching timestamps by
+ % target_file. It avoids having to compute a file name for a
+ % target_file first, before looking up the timestamp for that file.
+ TargetFileTimestamps0 = !.Info ^ mki_target_file_timestamps,
+ ( if map.search(TargetFileTimestamps0, TargetFile, Timestamp) then
+ MaybeTimestamp = ok(Timestamp)
+ else
+ get_file_name(Globals, Search, TargetFile, FileName, !Info, !IO),
+ get_target_timestamp_2(Globals, Search, TargetFile,
+ FileName, MaybeTimestamp, !Info, !IO),
+ (
+ MaybeTimestamp = ok(Timestamp),
+ TargetFileTimestamps1 = !.Info ^ mki_target_file_timestamps,
+ map.det_insert(TargetFile, Timestamp,
+ TargetFileTimestamps1, TargetFileTimestamps),
+ !Info ^ mki_target_file_timestamps := TargetFileTimestamps
+ ;
+ MaybeTimestamp = error(_)
+ % Do not record errors. These would usually be due to files not
+ % yet made, and the result would have to be updated once the
+ % file is made.
+ )
+ )
).
% Special treatment for `.analysis' files. If the corresponding
@@ -437,11 +458,10 @@ get_file_name(Globals, Search, TargetFile, FileName, !Info, !IO) :-
( if target_type_to_extension(Globals, TargetType, Ext) then
(
Search = do_search,
- module_name_to_search_file_name_cache(Globals, Ext,
- ModuleName, FileName, !Info, !IO)
+ module_name_to_search_file_name(Globals, $pred, Ext,
+ ModuleName, FileName, !IO)
;
Search = do_not_search,
- % Not common enough to cache.
module_name_to_file_name(Globals, $pred, do_not_create_dirs,
Ext, ModuleName, FileName, !IO)
)
@@ -451,23 +471,6 @@ get_file_name(Globals, Search, TargetFile, FileName, !Info, !IO) :-
)
).
-:- pred module_name_to_search_file_name_cache(globals::in, ext::in,
- module_name::in, string::out, make_info::in, make_info::out,
- io::di, io::uo) is det.
-
-module_name_to_search_file_name_cache(Globals, Ext, ModuleName, FileName,
- !Info, !IO) :-
- Key = module_name_ext(ModuleName, Ext),
- Cache0 = !.Info ^ mki_search_file_name_cache,
- ( if map.search(Cache0, Key, FileName0) then
- FileName = FileName0
- else
- module_name_to_search_file_name(Globals, $pred, Ext,
- ModuleName, FileName, !IO),
- map.det_insert(Key, FileName, Cache0, Cache),
- !Info ^ mki_search_file_name_cache := Cache
- ).
-
get_file_timestamp(SearchDirs, FileName, MaybeTimestamp, !Info, !IO) :-
FileTimestamps0 = !.Info ^ mki_file_timestamps,
( if map.search(FileTimestamps0, FileName, MaybeTimestamp0) then
@@ -562,7 +565,10 @@ make_remove_file(Globals, VerboseOption, FileName, !Info, !IO) :-
io.file.remove_file_recursively(FileName, _, !IO),
FileTimestamps0 = !.Info ^ mki_file_timestamps,
map.delete(FileName, FileTimestamps0, FileTimestamps),
- !Info ^ mki_file_timestamps := FileTimestamps.
+ !Info ^ mki_file_timestamps := FileTimestamps,
+
+ % For simplicity, clear out all target file timestamps.
+ !Info ^ mki_target_file_timestamps := map.init.
:- pred report_remove_file(string::in, io::di, io::uo) is det.
--
2.38.0
More information about the reviews
mailing list