[m-rev.] for review: Optimise dependency file index maps.
Zoltan Somogyi
zoltan.somogyi at runbox.com
Mon Nov 28 16:47:26 AEDT 2022
2022-11-28 15:06 GMT+11:00 "Peter Wang" <novalazy at gmail.com>:
> 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.
I didn't understand that sentence until I read the diff, which is not good
for something that is supposed to explain the diff.
> make.dependencies.of_3
This is unrelated to this diff, but if you feel you understand
of_2 and of_3's tasks well enough to write a comment explaining those
tasks, please do so.
> 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.
The two halves of that sentence don't go together.
The diff looks good, but I have a question about it. You add this new type
with the long name:
> +:- type dependency_file_with_module_index
> + ---> dfmi_target(module_index, module_target_type)
> + ; dfmi_file(file_name).
This new type is isomorphic to a flattened version of the existing dependency_file
type, with the module_name replaced with a module_index. Have you considered
an approach that, instead of adding this new type, just changes the args of the dep_target
cons_id of dependency_file with a triple: module_name, module_index, and module_target_type?
This would eliminate all the conversions, but would work only if you could assign
a module_index to every module_name you put into a dep_target. I think you can,
but then, I haven't tried to make it happen. If you can make it happen, I think that
approach would be better than the one in this diff. (The hash would of course ignore
the module name field.)
Zoltan.
More information about the reviews
mailing list