From 63e2517d39e679c0f8d23cac54311406d3ca243b Mon Sep 17 00:00:00 2001 From: Joel Rosdahl Date: Tue, 13 Dec 2022 20:32:15 +0100 Subject: [PATCH] fix: Fix reporting of local/remote cache misses in depend mode --- src/ccache.cpp | 5 +++ src/storage/Storage.cpp | 5 +++ src/storage/local/LocalStorage.cpp | 5 +++ test/suites/remote_file.bash | 55 ++++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+) diff --git a/src/ccache.cpp b/src/ccache.cpp index c063a58fe..8791f788e 100644 --- a/src/ccache.cpp +++ b/src/ccache.cpp @@ -631,6 +631,11 @@ process_preprocessed_file(Context& ctx, Hash& hash, const std::string& path) static std::optional result_key_from_depfile(Context& ctx, Hash& hash) { + // Make sure that result hash will always be different from the manifest hash + // since there otherwise may a storage key collision (in case the dependency + // file is empty). + hash.hash_delimiter("result"); + const auto file_content = util::read_file(ctx.args_info.output_dep); if (!file_content) { diff --git a/src/storage/Storage.cpp b/src/storage/Storage.cpp index 5c843db27..da707ea6e 100644 --- a/src/storage/Storage.cpp +++ b/src/storage/Storage.cpp @@ -463,6 +463,11 @@ Storage::get_from_remote_storage(const Digest& key, local.increment_statistic(core::Statistic::remote_storage_read_miss); if (type == core::CacheEntryType::result) { local.increment_statistic(core::Statistic::remote_storage_miss); + } else if (m_config.depend_mode()) { + // With the depend mode enabled, a missing manifest means that we can't + // even try to look up a result, so note the miss already now. + ASSERT(type == core::CacheEntryType::manifest); + local.increment_statistic(core::Statistic::remote_storage_miss); } } } diff --git a/src/storage/local/LocalStorage.cpp b/src/storage/local/LocalStorage.cpp index 274a44603..f1984caa8 100644 --- a/src/storage/local/LocalStorage.cpp +++ b/src/storage/local/LocalStorage.cpp @@ -223,6 +223,11 @@ LocalStorage::get(const Digest& key, const core::CacheEntryType type) if (type == core::CacheEntryType::result) { increment_statistic(return_value ? core::Statistic::local_storage_hit : core::Statistic::local_storage_miss); + } else if (m_config.depend_mode() && !return_value) { + // With the depend mode enabled, a missing manifest means that we can't even + // try to look up a result, so note the miss already now. + ASSERT(type == core::CacheEntryType::manifest); + increment_statistic(core::Statistic::local_storage_miss); } return return_value; diff --git a/test/suites/remote_file.bash b/test/suites/remote_file.bash index c4a8c1d29..a48ae561c 100644 --- a/test/suites/remote_file.bash +++ b/test/suites/remote_file.bash @@ -194,6 +194,61 @@ SUITE_remote_file() { expect_stat files_in_cache 4 expect_file_count 3 '*' remote # CACHEDIR.TAG + result + manifest + # ------------------------------------------------------------------------- + TEST "Depend mode" + + export CCACHE_DEPEND=1 + + # Compile and send result to local and remote storage. + $CCACHE_COMPILE -MMD -c test.c + expect_stat direct_cache_hit 0 + expect_stat cache_miss 1 + expect_stat files_in_cache 2 + expect_stat local_storage_hit 0 + expect_stat local_storage_miss 1 + expect_stat local_storage_read_hit 0 + expect_stat local_storage_read_miss 1 # only manifest + expect_stat local_storage_write 2 # result + manifest + expect_stat remote_storage_hit 0 + expect_stat remote_storage_miss 1 + expect_stat remote_storage_read_hit 0 + expect_stat remote_storage_read_miss 1 # only manifest + expect_stat remote_storage_write 2 # result + manifest + + # Get result from local storage. + $CCACHE_COMPILE -MMD -c test.c + expect_stat direct_cache_hit 1 + expect_stat cache_miss 1 + expect_stat local_storage_hit 1 + expect_stat local_storage_miss 1 + expect_stat local_storage_read_hit 2 # result + manifest + expect_stat local_storage_read_miss 1 # manifest + expect_stat local_storage_write 2 # result + manifest + expect_stat remote_storage_hit 0 + expect_stat remote_storage_miss 1 + expect_stat remote_storage_read_hit 0 + expect_stat remote_storage_read_miss 1 + expect_stat remote_storage_write 2 + + # Clear local storage. + $CCACHE -C >/dev/null + + # Get result from remote storage, copying it to local storage. + # TERM=xterm-256color gdb --args $CCACHE_COMPILE -MMD -c test.c + $CCACHE_COMPILE -MMD -c test.c + expect_stat direct_cache_hit 2 + expect_stat cache_miss 1 + expect_stat local_storage_hit 1 + expect_stat local_storage_miss 3 + expect_stat local_storage_read_hit 2 + expect_stat local_storage_read_miss 3 + expect_stat local_storage_write 4 + expect_stat remote_storage_hit 1 + expect_stat remote_storage_miss 1 + expect_stat remote_storage_read_hit 2 # result + manifest + expect_stat remote_storage_read_miss 1 + expect_stat remote_storage_write 2 # result + manifest + # ------------------------------------------------------------------------- TEST "umask" -- 2.47.2