From 0887dcaa24db74ef1e101827e3679d2ed0c3ae6e Mon Sep 17 00:00:00 2001 From: Joel Rosdahl Date: Sat, 8 Jun 2019 20:49:39 +0200 Subject: [PATCH] Improve naming of things MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Some words are being used to mean several things in the code base: * “object” can mean both “the object file (.o) produced by the compiler” and “the result stored in the cache, including e.g. the .o file and .d file”. * “hash” can mean both “the state that the hash_* functions operate on”, “the output of a hash function” and “the key used to index results and manifests in the cache”. This commits tries to make the naming more consistent: * “object” means “the object file (.o) produced by the compiler” * “result” means “the result stored in the cache, including e.g. the .o file and .d file”. * “struct hash” means “the state that the hash_* functions operate on”. * “digest” means “the output of a hash function”. However, “hash” is still used in documentation and command line output since I think that “hash” is easier to understand for most people, especially since that’s the term used by Git. * “name” means “the key used to index results and manifests in the cache”. --- src/ccache.c | 121 +++++++++++++++++++++--------------------- src/manifest.c | 139 +++++++++++++++++++++++++------------------------ src/manifest.h | 2 +- 3 files changed, 131 insertions(+), 131 deletions(-) diff --git a/src/ccache.c b/src/ccache.c index c47c2f2de..3e0e28a88 100644 --- a/src/ccache.c +++ b/src/ccache.c @@ -136,8 +136,8 @@ static size_t arch_args_size = 0; static char *arch_args[MAX_ARCH_ARGS] = {NULL}; // Name (represented as a struct digest) of the file containing the cached -// object code. -static struct digest *cached_obj_hash; +// result. +static struct digest *cached_result_name; // Full path to the file containing the result // (cachedir/a/b/cdef[...]-size.result). @@ -1110,7 +1110,7 @@ out: // Extract the used includes from the dependency file. Note that we cannot // distinguish system headers from other includes here. static struct digest * -object_hash_from_depfile(const char *depfile, struct hash *hash) +result_name_from_depfile(const char *depfile, struct hash *hash) { FILE *f = fopen(depfile, "r"); if (!f) { @@ -1188,8 +1188,8 @@ update_manifest_file(void) } MTR_BEGIN("manifest", "manifest_put"); - if (manifest_put(manifest_path, cached_obj_hash, included_files)) { - cc_log("Added object file hash to %s", manifest_path); + if (manifest_put(manifest_path, cached_result_name, included_files)) { + cc_log("Added result name to %s", manifest_path); if (x_stat(manifest_path, &st) == 0) { stats_update_size( manifest_stats_file, @@ -1197,19 +1197,19 @@ update_manifest_file(void) old_size == 0 ? 1 : 0); } } else { - cc_log("Failed to add object file hash to %s", manifest_path); + cc_log("Failed to add result name to %s", manifest_path); } MTR_END("manifest", "manifest_put"); } static void -update_cached_result_globals(struct digest *hash) +update_cached_result_globals(struct digest *result_name) { - char object_name[DIGEST_STRING_BUFFER_SIZE]; - digest_as_string(hash, object_name); - cached_obj_hash = hash; - cached_result = get_path_in_cache(object_name, ".result"); - stats_file = format("%s/%c/stats", conf->cache_dir, object_name[0]); + char result_name_string[DIGEST_STRING_BUFFER_SIZE]; + digest_as_string(result_name, result_name_string); + cached_result_name = result_name; + cached_result = get_path_in_cache(result_name_string, ".result"); + stats_file = format("%s/%c/stats", conf->cache_dir, result_name_string[0]); } // Run the real compiler and put the result in cache. @@ -1252,7 +1252,7 @@ to_cache(struct args *args, struct hash *depend_mode_hash) status = execute(args->argv, tmp_stdout_fd, tmp_stderr_fd, &compiler_pid); args_pop(args, 3); } else { - // The cached object path is not known yet, use temporary files. + // The cached result path is not known yet, use temporary files. tmp_stdout = format("%s/tmp.stdout", temp_dir()); tmp_stdout_fd = create_tmp_fd(&tmp_stdout); tmp_stderr = format("%s/tmp.stderr", temp_dir()); @@ -1349,12 +1349,12 @@ to_cache(struct args *args, struct hash *depend_mode_hash) } if (conf->depend_mode) { - struct digest *object_hash = - object_hash_from_depfile(output_dep, depend_mode_hash); - if (!object_hash) { + struct digest *result_name = + result_name_from_depfile(output_dep, depend_mode_hash); + if (!result_name) { failed(); } - update_cached_result_globals(object_hash); + update_cached_result_globals(result_name); } bool produce_dep_file = @@ -1452,10 +1452,10 @@ to_cache(struct args *args, struct hash *depend_mode_hash) free(tmp_stdout); } -// Find the object file name by running the compiler in preprocessor mode. -// Returns the hash as a heap-allocated hex string. +// Find the result name by running the compiler in preprocessor mode and +// hashing the result. static struct digest * -get_object_name_from_cpp(struct args *args, struct hash *hash) +get_result_name_from_cpp(struct args *args, struct hash *hash) { time_of_compilation = time(NULL); @@ -1643,10 +1643,9 @@ hash_nvcc_host_compiler(struct hash *hash, struct stat *ccbin_st, } } -// Update a hash sum with information common for the direct and preprocessor -// modes. +// Update a hash with information common for the direct and preprocessor modes. static void -calculate_common_hash(struct args *args, struct hash *hash) +hash_common_info(struct args *args, struct hash *hash) { hash_string(hash, HASH_PREFIX); @@ -1798,10 +1797,10 @@ calculate_common_hash(struct args *args, struct hash *hash) } // Update a hash sum with information specific to the direct and preprocessor -// modes and calculate the object hash. Returns the object hash on success, +// modes and calculate the result name. Returns the result name on success, // otherwise NULL. Caller frees. static struct digest * -calculate_object_hash(struct args *args, struct hash *hash, int direct_mode) +calculate_result_name(struct args *args, struct hash *hash, int direct_mode) { bool found_ccbin = false; @@ -2006,7 +2005,7 @@ calculate_object_hash(struct args *args, struct hash *hash, int direct_mode) hash_string(hash, arch_args[i]); } - struct digest *object_hash = NULL; + struct digest *result_name = NULL; if (direct_mode) { // Hash environment variables that affect the preprocessor output. const char *envvars[] = { @@ -2049,29 +2048,29 @@ calculate_object_hash(struct args *args, struct hash *hash, int direct_mode) manifest_stats_file = format("%s/%c/stats", conf->cache_dir, manifest_name[0]); - cc_log("Looking for object file hash in %s", manifest_path); + cc_log("Looking for result name in %s", manifest_path); MTR_BEGIN("manifest", "manifest_get"); - object_hash = manifest_get(conf, manifest_path); + result_name = manifest_get(conf, manifest_path); MTR_END("manifest", "manifest_get"); - if (object_hash) { - cc_log("Got object file hash from manifest"); + if (result_name) { + cc_log("Got result name from manifest"); } else { - cc_log("Did not find object file hash in manifest"); + cc_log("Did not find result name in manifest"); } } else { if (arch_args_size == 0) { - object_hash = get_object_name_from_cpp(args, hash); - cc_log("Got object file hash from preprocessor"); + result_name = get_result_name_from_cpp(args, hash); + cc_log("Got result name from preprocessor"); } else { args_add(args, "-arch"); for (size_t i = 0; i < arch_args_size; ++i) { args_add(args, arch_args[i]); - object_hash = get_object_name_from_cpp(args, hash); - cc_log("Got object file hash from preprocessor with -arch %s", + result_name = get_result_name_from_cpp(args, hash); + cc_log("Got result name from preprocessor with -arch %s", arch_args[i]); if (i != arch_args_size - 1) { - free(object_hash); - object_hash = NULL; + free(result_name); + result_name = NULL; } args_pop(args, 1); } @@ -2085,13 +2084,13 @@ calculate_object_hash(struct args *args, struct hash *hash, int direct_mode) } } - return object_hash; + return result_name; } // Try to return the compile result from cache. If we can return from cache // then this function exits with the correct status code, otherwise it returns. static void -from_cache(enum fromcache_call_mode mode, bool put_object_in_manifest) +from_cache(enum fromcache_call_mode mode, bool put_result_in_manifest) { // The user might be disabling cache hits. if (conf->recache) { @@ -2168,7 +2167,7 @@ from_cache(enum fromcache_call_mode mode, bool put_object_in_manifest) send_cached_stderr(tmp_stderr); - if (put_object_in_manifest) { + if (put_result_in_manifest) { update_manifest_file(); } @@ -3566,7 +3565,7 @@ cc_reset(void) free(output_su); output_su = NULL; free(output_dia); output_dia = NULL; free(output_dwo); output_dwo = NULL; - free(cached_obj_hash); cached_obj_hash = NULL; + free(cached_result_name); cached_result_name = NULL; free(cached_result); cached_result = NULL; free(manifest_path); manifest_path = NULL; time_of_compilation = 0; @@ -3728,7 +3727,7 @@ ccache(int argc, char *argv[]) init_hash_debug(common_hash, output_obj, 'c', "COMMON", debug_text_file); MTR_BEGIN("hash", "common_hash"); - calculate_common_hash(preprocessor_args, common_hash); + hash_common_info(preprocessor_args, common_hash); MTR_END("hash", "common_hash"); // Try to find the hash using the manifest. @@ -3736,28 +3735,28 @@ ccache(int argc, char *argv[]) init_hash_debug( direct_hash, output_obj, 'd', "DIRECT MODE", debug_text_file); - bool put_object_in_manifest = false; - struct digest *object_hash = NULL; - struct digest *object_hash_from_manifest = NULL; + bool put_result_in_manifest = false; + struct digest *result_name = NULL; + struct digest *result_name_from_manifest = NULL; if (conf->direct_mode) { cc_log("Trying direct lookup"); MTR_BEGIN("hash", "direct_hash"); - object_hash = calculate_object_hash(preprocessor_args, direct_hash, 1); + result_name = calculate_result_name(preprocessor_args, direct_hash, 1); MTR_END("hash", "direct_hash"); - if (object_hash) { - update_cached_result_globals(object_hash); + if (result_name) { + update_cached_result_globals(result_name); // If we can return from cache at this point then do so. from_cache(FROMCACHE_DIRECT_MODE, 0); - // Wasn't able to return from cache at this point. However, the object + // Wasn't able to return from cache at this point. However, the result // was already found in manifest, so don't re-add it later. - put_object_in_manifest = false; + put_result_in_manifest = false; - object_hash_from_manifest = object_hash; + result_name_from_manifest = result_name; } else { - // Add object to manifest later. - put_object_in_manifest = true; + // Add result to manifest later. + put_result_in_manifest = true; } } @@ -3774,15 +3773,15 @@ ccache(int argc, char *argv[]) cpp_hash, output_obj, 'p', "PREPROCESSOR MODE", debug_text_file); MTR_BEGIN("hash", "cpp_hash"); - object_hash = calculate_object_hash(preprocessor_args, cpp_hash, 0); + result_name = calculate_result_name(preprocessor_args, cpp_hash, 0); MTR_END("hash", "cpp_hash"); - if (!object_hash) { - fatal("internal error: object hash from cpp returned NULL"); + if (!result_name) { + fatal("internal error: calculate_result_name returned NULL for cpp"); } - update_cached_result_globals(object_hash); + update_cached_result_globals(result_name); - if (object_hash_from_manifest - && !digests_equal(object_hash_from_manifest, object_hash)) { + if (result_name_from_manifest + && !digests_equal(result_name_from_manifest, result_name)) { // The hash from manifest differs from the hash of the preprocessor // output. This could be because: // @@ -3800,11 +3799,11 @@ ccache(int argc, char *argv[]) cc_log("Removing manifest as a safety measure"); x_unlink(manifest_path); - put_object_in_manifest = true; + put_result_in_manifest = true; } // If we can return from cache at this point then do. - from_cache(FROMCACHE_CPP_MODE, put_object_in_manifest); + from_cache(FROMCACHE_CPP_MODE, put_result_in_manifest); } if (conf->read_only) { diff --git a/src/manifest.c b/src/manifest.c index 1de248a45..00da5424e 100644 --- a/src/manifest.c +++ b/src/manifest.c @@ -74,12 +74,12 @@ struct file_info { int64_t ctime; }; -struct object { +struct result { // Number of entries in file_info_indexes. uint32_t n_file_info_indexes; // Indexes to file_infos. uint32_t *file_info_indexes; - // Name of the object itself. + // Name of the result. struct digest name; }; @@ -95,9 +95,9 @@ struct manifest { uint32_t n_file_infos; struct file_info *file_infos; - // Object names plus references to include file hashes. - uint32_t n_objects; - struct object *objects; + // Result names plus references to include file infos. + uint32_t n_results; + struct result *results; }; struct file_stats { @@ -133,10 +133,10 @@ free_manifest(struct manifest *mf) } free(mf->files); free(mf->file_infos); - for (uint32_t i = 0; i < mf->n_objects; i++) { - free(mf->objects[i].file_info_indexes); + for (uint32_t i = 0; i < mf->n_results; i++) { + free(mf->results[i].file_info_indexes); } - free(mf->objects); + free(mf->results); free(mf); } @@ -202,8 +202,8 @@ create_empty_manifest(void) mf->files = NULL; mf->n_file_infos = 0; mf->file_infos = NULL; - mf->n_objects = 0; - mf->objects = NULL; + mf->n_results = 0; + mf->results = NULL; return mf; } @@ -250,17 +250,17 @@ read_manifest(gzFile f, char **errmsg) READ_INT(8, mf->file_infos[i].ctime); } - READ_INT(4, mf->n_objects); - mf->objects = x_calloc(mf->n_objects, sizeof(*mf->objects)); - for (uint32_t i = 0; i < mf->n_objects; i++) { - READ_INT(4, mf->objects[i].n_file_info_indexes); - mf->objects[i].file_info_indexes = - x_calloc(mf->objects[i].n_file_info_indexes, - sizeof(*mf->objects[i].file_info_indexes)); - for (uint32_t j = 0; j < mf->objects[i].n_file_info_indexes; j++) { - READ_INT(4, mf->objects[i].file_info_indexes[j]); + READ_INT(4, mf->n_results); + mf->results = x_calloc(mf->n_results, sizeof(*mf->results)); + for (uint32_t i = 0; i < mf->n_results; i++) { + READ_INT(4, mf->results[i].n_file_info_indexes); + mf->results[i].file_info_indexes = + x_calloc(mf->results[i].n_file_info_indexes, + sizeof(*mf->results[i].file_info_indexes)); + for (uint32_t j = 0; j < mf->results[i].n_file_info_indexes; j++) { + READ_INT(4, mf->results[i].file_info_indexes[j]); } - READ_BYTES(DIGEST_SIZE, mf->objects[i].name.bytes); + READ_BYTES(DIGEST_SIZE, mf->results[i].name.bytes); } return mf; @@ -325,13 +325,13 @@ write_manifest(gzFile f, const struct manifest *mf) WRITE_INT(8, mf->file_infos[i].ctime); } - WRITE_INT(4, mf->n_objects); - for (uint32_t i = 0; i < mf->n_objects; i++) { - WRITE_INT(4, mf->objects[i].n_file_info_indexes); - for (uint32_t j = 0; j < mf->objects[i].n_file_info_indexes; j++) { - WRITE_INT(4, mf->objects[i].file_info_indexes[j]); + WRITE_INT(4, mf->n_results); + for (uint32_t i = 0; i < mf->n_results; i++) { + WRITE_INT(4, mf->results[i].n_file_info_indexes); + for (uint32_t j = 0; j < mf->results[i].n_file_info_indexes; j++) { + WRITE_INT(4, mf->results[i].file_info_indexes[j]); } - WRITE_BYTES(DIGEST_SIZE, mf->objects[i].name.bytes); + WRITE_BYTES(DIGEST_SIZE, mf->results[i].name.bytes); } return 1; @@ -341,18 +341,18 @@ error: return 0; } -static int -verify_object(struct conf *conf, struct manifest *mf, struct object *obj, +static bool +verify_result(struct conf *conf, struct manifest *mf, struct result *result, struct hashtable *stated_files, struct hashtable *hashed_files) { - for (uint32_t i = 0; i < obj->n_file_info_indexes; i++) { - struct file_info *fi = &mf->file_infos[obj->file_info_indexes[i]]; + for (uint32_t i = 0; i < result->n_file_info_indexes; i++) { + struct file_info *fi = &mf->file_infos[result->file_info_indexes[i]]; char *path = mf->files[fi->index]; struct file_stats *st = hashtable_search(stated_files, path); if (!st) { struct stat file_stat; if (x_stat(path, &file_stat) != 0) { - return 0; + return false; } st = x_malloc(sizeof(*st)); st->size = file_stat.st_size; @@ -362,7 +362,7 @@ verify_object(struct conf *conf, struct manifest *mf, struct object *obj, } if (fi->fsize != st->size) { - return 0; + return false; } // Clang stores the mtime of the included files in the precompiled header, @@ -372,7 +372,7 @@ verify_object(struct conf *conf, struct manifest *mf, struct object *obj, && output_is_precompiled_header && fi->mtime != st->mtime) { cc_log("Precompiled header includes %s, which has a new mtime", path); - return 0; + return false; } if (conf->sloppiness & SLOPPY_FILE_STAT_MATCHES) { @@ -396,19 +396,19 @@ verify_object(struct conf *conf, struct manifest *mf, struct object *obj, struct digest *actual = hashtable_search(hashed_files, path); if (!actual) { struct hash *hash = hash_init(); - int result = hash_source_code_file(conf, hash, path); - if (result & HASH_SOURCE_CODE_ERROR) { + int ret = hash_source_code_file(conf, hash, path); + if (ret & HASH_SOURCE_CODE_ERROR) { cc_log("Failed hashing %s", path); hash_free(hash); - return 0; + return false; } - if (result & HASH_SOURCE_CODE_FOUND_TIME) { + if (ret & HASH_SOURCE_CODE_FOUND_TIME) { hash_free(hash); - return 0; + return false; } + actual = malloc(sizeof(*actual)); hash_result_as_bytes(hash, actual); - hashtable_insert(hashed_files, x_strdup(path), actual); hash_free(hash); } @@ -417,7 +417,7 @@ verify_object(struct conf *conf, struct manifest *mf, struct object *obj, } } - return 1; + return true; } static struct hashtable * @@ -465,7 +465,7 @@ get_include_file_index(struct manifest *mf, char *path, } static uint32_t -get_file_hash_index(struct manifest *mf, +get_file_info_index(struct manifest *mf, char *path, struct digest *digest, struct hashtable *mf_files, @@ -529,7 +529,7 @@ add_file_info_indexes(uint32_t *indexes, uint32_t size, do { char *path = hashtable_iterator_key(iter); struct digest *digest = hashtable_iterator_value(iter); - indexes[i] = get_file_hash_index(mf, path, digest, mf_files, + indexes[i] = get_file_info_index(mf, path, digest, mf_files, mf_file_infos); i++; } while (hashtable_iterator_advance(iter)); @@ -540,23 +540,24 @@ add_file_info_indexes(uint32_t *indexes, uint32_t size, } static void -add_object_entry(struct manifest *mf, - struct digest *object_hash, +add_result_entry(struct manifest *mf, + struct digest *result_digest, struct hashtable *included_files) { - uint32_t n_objs = mf->n_objects; - mf->objects = x_realloc(mf->objects, (n_objs + 1) * sizeof(*mf->objects)); - mf->n_objects++; - struct object *obj = &mf->objects[n_objs]; + uint32_t n_results = mf->n_results; + mf->results = x_realloc(mf->results, (n_results + 1) * sizeof(*mf->results)); + mf->n_results++; + struct result *result = &mf->results[n_results]; uint32_t n_fii = hashtable_count(included_files); - obj->n_file_info_indexes = n_fii; - obj->file_info_indexes = x_malloc(n_fii * sizeof(*obj->file_info_indexes)); - add_file_info_indexes(obj->file_info_indexes, n_fii, mf, included_files); - obj->name = *object_hash; + result->n_file_info_indexes = n_fii; + result->file_info_indexes = + x_malloc(n_fii * sizeof(*result->file_info_indexes)); + add_file_info_indexes(result->file_info_indexes, n_fii, mf, included_files); + result->name = *result_digest; } -// Try to get the object hash from a manifest file. Caller frees. Returns NULL +// Try to get the result name from a manifest file. Caller frees. Returns NULL // on failure. struct digest * manifest_get(struct conf *conf, const char *manifest_path) @@ -590,12 +591,12 @@ manifest_get(struct conf *conf, const char *manifest_path) hashed_files = create_hashtable(1000, hash_from_string, strings_equal); stated_files = create_hashtable(1000, hash_from_string, strings_equal); - // Check newest object first since it's a bit more likely to match. - for (uint32_t i = mf->n_objects; i > 0; i--) { - if (verify_object(conf, mf, &mf->objects[i - 1], + // Check newest result first since it's a bit more likely to match. + for (uint32_t i = mf->n_results; i > 0; i--) { + if (verify_result(conf, mf, &mf->results[i - 1], stated_files, hashed_files)) { name = x_malloc(sizeof(*name)); - *name = mf->objects[i - 1].name; + *name = mf->results[i - 1].name; goto out; } } @@ -620,10 +621,10 @@ out: return name; } -// Put the object name into a manifest file given a set of included files. +// Put the result name into a manifest file given a set of included files. // Returns true on success, otherwise false. bool -manifest_put(const char *manifest_path, struct digest *object_hash, +manifest_put(const char *manifest_path, struct digest *result_name, struct hashtable *included_files) { int ret = 0; @@ -658,15 +659,15 @@ manifest_put(const char *manifest_path, struct digest *object_hash, } } - if (mf->n_objects > MAX_MANIFEST_ENTRIES) { - // Normally, there shouldn't be many object entries in the manifest since + if (mf->n_results > MAX_MANIFEST_ENTRIES) { + // Normally, there shouldn't be many result entries in the manifest since // new entries are added only if an include file has changed but not the // source file, and you typically change source files more often than // header files. However, it's certainly possible to imagine cases where // the manifest will grow large (for instance, a generated header file that // changes for every build), and this must be taken care of since // processing an ever growing manifest eventually will take too much time. - // A good way of solving this would be to maintain the object entries in + // A good way of solving this would be to maintain the result entries in // LRU order and discarding the old ones. An easy way is to throw away all // entries when there are too many. Let's do that for now. cc_log("More than %u entries in manifest file; discarding", @@ -691,7 +692,7 @@ manifest_put(const char *manifest_path, struct digest *object_hash, goto out; } - add_object_entry(mf, object_hash, included_files); + add_result_entry(mf, result_name, included_files); if (write_manifest(f2, mf)) { gzclose(f2); f2 = NULL; @@ -766,17 +767,17 @@ manifest_dump(const char *manifest_path, FILE *stream) fprintf(stream, " Mtime: %lld\n", (long long)mf->file_infos[i].mtime); fprintf(stream, " Ctime: %lld\n", (long long)mf->file_infos[i].ctime); } - fprintf(stream, "Results (%u):\n", (unsigned)mf->n_objects); - for (unsigned i = 0; i < mf->n_objects; ++i) { + fprintf(stream, "Results (%u):\n", (unsigned)mf->n_results); + for (unsigned i = 0; i < mf->n_results; ++i) { char name[DIGEST_STRING_BUFFER_SIZE]; fprintf(stream, " %u:\n", i); fprintf(stream, " File info indexes:"); - for (unsigned j = 0; j < mf->objects[i].n_file_info_indexes; ++j) { - fprintf(stream, " %u", mf->objects[i].file_info_indexes[j]); + for (unsigned j = 0; j < mf->results[i].n_file_info_indexes; ++j) { + fprintf(stream, " %u", mf->results[i].file_info_indexes[j]); } fprintf(stream, "\n"); - digest_as_string(&mf->objects[i].name, name); - fprintf(stream, " Hash: %s\n", name); + digest_as_string(&mf->results[i].name, name); + fprintf(stream, " Name: %s\n", name); } ret = true; diff --git a/src/manifest.h b/src/manifest.h index ff2aca622..1c78cf01a 100644 --- a/src/manifest.h +++ b/src/manifest.h @@ -8,7 +8,7 @@ #define MANIFEST_VERSION 2 struct digest *manifest_get(struct conf *conf, const char *manifest_path); -bool manifest_put(const char *manifest_path, struct digest *object_hash, +bool manifest_put(const char *manifest_path, struct digest *result_digest, struct hashtable *included_files); bool manifest_dump(const char *manifest_path, FILE *stream); -- 2.47.2