> This patch implements Incremental LTO as ltrans cache.
>
> The cache is active when directory $GCC_LTRANS_CACHE is specified and exists.
> Stored are pairs of ltrans input/output files and input file hash.
> File locking is used to allow multiple GCC instances to use to same cache.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu
>
> gcc/ChangeLog:
>
> * Makefile.in: Add lto-ltrans-cache.o.
> * lto-wrapper.cc: Use ltrans cache.
> * lto-ltrans-cache.cc: New file.
> * lto-ltrans-cache.h: New file.
> diff --git a/gcc/lto-ltrans-cache.cc b/gcc/lto-ltrans-cache.cc
> new file mode 100644
> index 00000000000..0d43e548fb3
> --- /dev/null
> +++ b/gcc/lto-ltrans-cache.cc
> @@ -0,0 +1,407 @@
> +/* File caching.
> + Copyright (C) 2009-2023 Free Software Foundation, Inc.
Probably copyright should be 2023-2024
> +const md5_checksum_t INVALID_CHECKSUM = {
Maybe static here? Officially there should be comment before the
function.
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> +};
> +
> +/* Computes checksum for given file, returns INVALID_CHECKSUM if not
> possible.
> + */
comment would look more regular if linebreak is made before possible :)
> +
> +/* Checks identity of two files byte by byte. */
> +static bool
> +files_identical (char const *first_filename, char const *second_filename)
> +{
> + FILE *f_first = fopen (first_filename, "rb");
> + if (!f_first)
> + return false;
> +
> + FILE *f_second = fopen (second_filename, "rb");
> + if (!f_second)
> + {
> + fclose (f_first);
> + return false;
> + }
> +
> + bool ret = true;
> +
> + for (;;)
> + {
> + int c1, c2;
> + c1 = fgetc (f_first);
> + c2 = fgetc (f_second);
I guess reading by fgetc may get quite ineffecient here. Comparing
bigger blocks is probably going to be faster. We could also
(incrementally) use mmap where supported.
> +
> +/* Contructor of cache item. */
> +ltrans_file_cache::item::item (std::string input, std::string output,
> + md5_checksum_t input_checksum, uint32_t last_used):
Here should be enough whitespace so md5_checksum appears just after ( in
line above
md5_checksum_t input_checksum, uint32_t
last_used):
> + input (std::move (input)), output (std::move (output)),
> + input_checksum (input_checksum), last_used (last_used)
> +{
> + lock = lockfile (this->input + ".lock");
> +}
> +/* Destructor of cache item. */
> +ltrans_file_cache::item::~item ()
> +{
> + lock.unlock ();
> +}
> +
> +/* Reads next cache item from cachedata file.
> + Adds `dir/` prefix to filenames. */
> +static ltrans_file_cache::item*
> +read_cache_item (FILE* f, const char* dir)
> +{
> + md5_checksum_t checksum;
> + uint32_t last_used;
> +
> + if (fread (&checksum, 1, checksum.size (), f) != checksum.size ())
> + return NULL;
> + if (fread (&last_used, sizeof (last_used), 1, f) != 1)
> + return NULL;
> +
> + std::vector<char> input (strlen (dir));
> + memcpy (&input[0], dir, input.size ());
> + input.push_back ('/');
Why this is not std::string?
> + /* Loads data about previously cached items from cachedata file.
> +
> + Must be called with creation_lock or deletion_lock held to
> + prevent data race. */
> + void
> + load_cache ();
There should be no newline between type and name. It is there only when
defining function (so it is easy to use old-school grep to find where
function is defined.)
Looks good to me otherwise.
Honza
