From 8c393673b2c3b8abeac99f0399a493b32f230761 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Mon, 2 Oct 2023 13:41:51 -0400 Subject: [PATCH] sccache now tracks compiler hits/misses on a per language basis. Previously sccache was actually tracking hits and misses on a compiler id basis, and guessing on the language. This is an issue for compilers like `clang` which support C, C++, and CUDA. In that case we want to clearly state what language we are getting hit or misses on to help user diagnose issues. --- src/compiler/c.rs | 55 +++----------------------- src/compiler/clang.rs | 8 ++-- src/compiler/compiler.rs | 71 +++++++++++++++++++++++++++++++--- src/compiler/diab.rs | 6 +-- src/compiler/gcc.rs | 8 ++-- src/compiler/msvc.rs | 6 +-- src/compiler/nvcc.rs | 8 ++-- src/compiler/nvhpc.rs | 8 ++-- src/compiler/rust.rs | 6 ++- src/compiler/tasking_vx.rs | 4 +- src/server.rs | 21 +++++----- tests/system.rs | 79 ++++++++++++++++++++++++++++++++++++-- tests/test_c.cu | 10 +++++ 13 files changed, 195 insertions(+), 95 deletions(-) create mode 100644 tests/test_c.cu diff --git a/src/compiler/c.rs b/src/compiler/c.rs index 68ef91e6aa..ed28fe5ca2 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -15,7 +15,7 @@ use crate::cache::FileObjectSource; use crate::compiler::{ Cacheable, ColorMode, Compilation, CompileCommand, Compiler, CompilerArguments, CompilerHasher, - CompilerKind, HashResult, + CompilerKind, HashResult, Language, }; #[cfg(feature = "dist-client")] use crate::compiler::{DistPackagers, NoopOutputsRewriter}; @@ -62,18 +62,6 @@ where compiler: I, } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum Language { - C, - Cxx, - GenericHeader, - CHeader, - CxxHeader, - ObjectiveC, - ObjectiveCxx, - Cuda, -} - /// Artifact produced by a C/C++ compiler. #[derive(Clone, Debug, PartialEq, Eq)] pub struct ArtifactDescriptor { @@ -129,43 +117,6 @@ impl ParsedArguments { } } -impl Language { - pub fn from_file_name(file: &Path) -> Option { - match file.extension().and_then(|e| e.to_str()) { - // gcc: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html - Some("c") => Some(Language::C), - // Could be C or C++ - Some("h") => Some(Language::GenericHeader), - // TODO i - Some("C") | Some("cc") | Some("cp") | Some("cpp") | Some("CPP") | Some("cxx") - | Some("c++") => Some(Language::Cxx), - // TODO ii - Some("H") | Some("hh") | Some("hp") | Some("hpp") | Some("HPP") | Some("hxx") - | Some("h++") | Some("tcc") => Some(Language::CxxHeader), - Some("m") => Some(Language::ObjectiveC), - // TODO mi - Some("M") | Some("mm") => Some(Language::ObjectiveCxx), - // TODO mii - Some("cu") => Some(Language::Cuda), - e => { - trace!("Unknown source extension: {}", e.unwrap_or("(None)")); - None - } - } - } - - pub fn as_str(self) -> &'static str { - match self { - Language::C | Language::CHeader => "c", - Language::Cxx | Language::CxxHeader => "c++", - Language::GenericHeader => "c/c++", - Language::ObjectiveC => "objc", - Language::ObjectiveCxx => "objc++", - Language::Cuda => "cuda", - } - } -} - /// A generic implementation of the `Compilation` trait for C/C++ compilers. struct CCompilation { parsed_args: ParsedArguments, @@ -443,6 +394,10 @@ where fn box_clone(&self) -> Box> { Box::new((*self).clone()) } + + fn language(&self) -> Language { + self.parsed_args.language + } } impl Compilation for CCompilation { diff --git a/src/compiler/clang.rs b/src/compiler/clang.rs index 7d7e535965..d1cc49d9e8 100644 --- a/src/compiler/clang.rs +++ b/src/compiler/clang.rs @@ -15,11 +15,11 @@ #![allow(unused_imports, dead_code, unused_variables)] use crate::compiler::args::*; -use crate::compiler::c::{ - ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments, -}; +use crate::compiler::c::{ArtifactDescriptor, CCompilerImpl, CCompilerKind, ParsedArguments}; use crate::compiler::gcc::ArgData::*; -use crate::compiler::{gcc, write_temp_file, Cacheable, CompileCommand, CompilerArguments}; +use crate::compiler::{ + gcc, write_temp_file, Cacheable, CompileCommand, CompilerArguments, Language, +}; use crate::mock_command::{CommandCreator, CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; use crate::{counted_array, dist}; diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 5da63a19f6..fe6a72a4ac 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -102,12 +102,71 @@ pub enum CompilerKind { Rust, } -impl CompilerKind { - pub fn lang_kind(&self) -> String { +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum Language { + C, + Cxx, + GenericHeader, + CHeader, + CxxHeader, + ObjectiveC, + ObjectiveCxx, + Cuda, + Rust, +} + +impl Language { + pub fn from_file_name(file: &Path) -> Option { + match file.extension().and_then(|e| e.to_str()) { + // gcc: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html + Some("c") => Some(Language::C), + // Could be C or C++ + Some("h") => Some(Language::GenericHeader), + // TODO i + Some("C") | Some("cc") | Some("cp") | Some("cpp") | Some("CPP") | Some("cxx") + | Some("c++") => Some(Language::Cxx), + // TODO ii + Some("H") | Some("hh") | Some("hp") | Some("hpp") | Some("HPP") | Some("hxx") + | Some("h++") | Some("tcc") => Some(Language::CxxHeader), + Some("m") => Some(Language::ObjectiveC), + // TODO mi + Some("M") | Some("mm") => Some(Language::ObjectiveCxx), + // TODO mii + Some("cu") => Some(Language::Cuda), + // TODO cy + Some("rs") => Some(Language::Rust), + e => { + trace!("Unknown source extension: {}", e.unwrap_or("(None)")); + None + } + } + } + + pub fn as_str(self) -> &'static str { match self { - CompilerKind::C(CCompilerKind::Nvcc) => "CUDA", - CompilerKind::C(_) => "C/C++", - CompilerKind::Rust => "Rust", + Language::C | Language::CHeader => "c", + Language::Cxx | Language::CxxHeader => "c++", + Language::GenericHeader => "c/c++", + Language::ObjectiveC => "objc", + Language::ObjectiveCxx => "objc++", + Language::Cuda => "cuda", + Language::Rust => "rust", + } + } +} + +impl CompilerKind { + pub fn lang_kind(&self, lang: &Language) -> String { + match lang { + Language::C + | Language::CHeader + | Language::Cxx + | Language::CxxHeader + | Language::GenericHeader + | Language::ObjectiveC + | Language::ObjectiveCxx => "C/C++", + Language::Cuda => "CUDA", + Language::Rust => "Rust", } .to_string() } @@ -427,6 +486,8 @@ where fn output_pretty(&self) -> Cow<'_, str>; fn box_clone(&self) -> Box>; + + fn language(&self) -> Language; } #[cfg(not(feature = "dist-client"))] diff --git a/src/compiler/diab.rs b/src/compiler/diab.rs index ef73f5d176..4eb3f410fb 100644 --- a/src/compiler/diab.rs +++ b/src/compiler/diab.rs @@ -17,10 +17,8 @@ use crate::compiler::args::{ ArgDisposition, ArgInfo, ArgToStringResult, ArgsIter, Argument, FromArg, IntoArg, NormalizedDisposition, PathTransformerFn, SearchableArgInfo, }; -use crate::compiler::c::{ - ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments, -}; -use crate::compiler::{Cacheable, ColorMode, CompileCommand, CompilerArguments}; +use crate::compiler::c::{ArtifactDescriptor, CCompilerImpl, CCompilerKind, ParsedArguments}; +use crate::compiler::{Cacheable, ColorMode, CompileCommand, CompilerArguments, Language}; use crate::errors::*; use crate::mock_command::{CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index 6991a50bad..80d8edbb71 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -13,10 +13,8 @@ // limitations under the License. use crate::compiler::args::*; -use crate::compiler::c::{ - ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments, -}; -use crate::compiler::{clang, Cacheable, ColorMode, CompileCommand, CompilerArguments}; +use crate::compiler::c::{ArtifactDescriptor, CCompilerImpl, CCompilerKind, ParsedArguments}; +use crate::compiler::{clang, Cacheable, ColorMode, CompileCommand, CompilerArguments, Language}; use crate::mock_command::{CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; use crate::{counted_array, dist}; @@ -367,6 +365,7 @@ where "objective-c" => Some(Language::ObjectiveC), "objective-c++" => Some(Language::ObjectiveCxx), "cu" => Some(Language::Cuda), + "rs" => Some(Language::Rust), _ => cannot_cache!("-x"), }; } @@ -615,6 +614,7 @@ fn language_to_gcc_arg(lang: Language) -> Option<&'static str> { Language::ObjectiveC => Some("objective-c"), Language::ObjectiveCxx => Some("objective-c++"), Language::Cuda => Some("cu"), + Language::Rust => None, // Let the compiler decide Language::GenericHeader => None, // Let the compiler decide } } diff --git a/src/compiler/msvc.rs b/src/compiler/msvc.rs index 8c0a24c5ba..6f6502b835 100644 --- a/src/compiler/msvc.rs +++ b/src/compiler/msvc.rs @@ -13,11 +13,9 @@ // limitations under the License. use crate::compiler::args::*; -use crate::compiler::c::{ - ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments, -}; +use crate::compiler::c::{ArtifactDescriptor, CCompilerImpl, CCompilerKind, ParsedArguments}; use crate::compiler::{ - clang, gcc, write_temp_file, Cacheable, ColorMode, CompileCommand, CompilerArguments, + clang, gcc, write_temp_file, Cacheable, ColorMode, CompileCommand, CompilerArguments, Language, }; use crate::mock_command::{CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; diff --git a/src/compiler/nvcc.rs b/src/compiler/nvcc.rs index 198b7ab29a..4d765aa5dd 100644 --- a/src/compiler/nvcc.rs +++ b/src/compiler/nvcc.rs @@ -16,11 +16,11 @@ #![allow(unused_imports, dead_code, unused_variables)] use crate::compiler::args::*; -use crate::compiler::c::{ - ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments, -}; +use crate::compiler::c::{ArtifactDescriptor, CCompilerImpl, CCompilerKind, ParsedArguments}; use crate::compiler::gcc::ArgData::*; -use crate::compiler::{gcc, write_temp_file, Cacheable, CompileCommand, CompilerArguments}; +use crate::compiler::{ + gcc, write_temp_file, Cacheable, CompileCommand, CompilerArguments, Language, +}; use crate::mock_command::{CommandCreator, CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; use crate::{counted_array, dist}; diff --git a/src/compiler/nvhpc.rs b/src/compiler/nvhpc.rs index 71055e3e35..247b2a4253 100644 --- a/src/compiler/nvhpc.rs +++ b/src/compiler/nvhpc.rs @@ -16,11 +16,11 @@ #![allow(unused_imports, dead_code, unused_variables)] use crate::compiler::args::*; -use crate::compiler::c::{ - ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments, -}; +use crate::compiler::c::{ArtifactDescriptor, CCompilerImpl, CCompilerKind, ParsedArguments}; use crate::compiler::gcc::ArgData::*; -use crate::compiler::{gcc, write_temp_file, Cacheable, CompileCommand, CompilerArguments}; +use crate::compiler::{ + gcc, write_temp_file, Cacheable, CompileCommand, CompilerArguments, Language, +}; use crate::mock_command::{CommandCreator, CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; use crate::{counted_array, dist}; diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 23e310e4cb..853b2a678b 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -16,7 +16,7 @@ use crate::cache::FileObjectSource; use crate::compiler::args::*; use crate::compiler::{ c::ArtifactDescriptor, Cacheable, ColorMode, Compilation, CompileCommand, Compiler, - CompilerArguments, CompilerHasher, CompilerKind, CompilerProxy, HashResult, + CompilerArguments, CompilerHasher, CompilerKind, CompilerProxy, HashResult, Language, }; #[cfg(feature = "dist-client")] use crate::compiler::{DistPackagers, OutputsRewriter}; @@ -1589,6 +1589,10 @@ where fn box_clone(&self) -> Box> { Box::new((*self).clone()) } + + fn language(&self) -> Language { + Language::Rust + } } impl Compilation for RustCompilation { diff --git a/src/compiler/tasking_vx.rs b/src/compiler/tasking_vx.rs index 14120cebe4..4ea06ca9c1 100644 --- a/src/compiler/tasking_vx.rs +++ b/src/compiler/tasking_vx.rs @@ -19,8 +19,8 @@ use crate::{ ArgDisposition, ArgInfo, ArgToStringResult, ArgsIter, Argument, FromArg, IntoArg, NormalizedDisposition, PathTransformerFn, SearchableArgInfo, }, - c::{ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments}, - Cacheable, ColorMode, CompileCommand, CompilerArguments, + c::{ArtifactDescriptor, CCompilerImpl, CCompilerKind, ParsedArguments}, + Cacheable, ColorMode, CompileCommand, CompilerArguments, Language, }, counted_array, dist, errors::*, diff --git a/src/server.rs b/src/server.rs index 594a9150b0..342ced1cd3 100644 --- a/src/server.rs +++ b/src/server.rs @@ -15,7 +15,7 @@ use crate::cache::{storage_from_config, Storage}; use crate::compiler::{ get_compiler_info, CacheControl, CompileResult, Compiler, CompilerArguments, CompilerHasher, - CompilerKind, CompilerProxy, DistType, MissType, + CompilerKind, CompilerProxy, DistType, Language, MissType, }; #[cfg(feature = "dist-client")] use crate::config; @@ -1164,6 +1164,7 @@ where let color_mode = hasher.color_mode(); let me = self.clone(); let kind = compiler.kind(); + let lang = hasher.language(); let creator = self.creator.clone(); let storage = self.storage.clone(); let pool = self.rt.clone(); @@ -1199,12 +1200,12 @@ where CompileResult::Error => { debug!("compile result: cache error"); - stats.cache_errors.increment(&kind); + stats.cache_errors.increment(&kind, &lang); } CompileResult::CacheHit(duration) => { debug!("compile result: cache hit"); - stats.cache_hits.increment(&kind); + stats.cache_hits.increment(&kind, &lang); stats.cache_read_hit_duration += duration; } CompileResult::CacheMiss(miss_type, dist_type, duration, future) => { @@ -1229,10 +1230,10 @@ where stats.cache_timeouts += 1; } MissType::CacheReadError => { - stats.cache_errors.increment(&kind); + stats.cache_errors.increment(&kind, &lang); } } - stats.cache_misses.increment(&kind); + stats.cache_misses.increment(&kind, &lang); stats.compiler_write_duration += duration; debug!("stats after compile result: {stats:?}"); cache_write = Some(future); @@ -1240,7 +1241,7 @@ where CompileResult::NotCacheable => { debug!("compile result: not cacheable"); - stats.cache_misses.increment(&kind); + stats.cache_misses.increment(&kind, &lang); stats.non_cacheable_compilations += 1; } CompileResult::CompileFailed => { @@ -1298,7 +1299,7 @@ where error!("[{:?}] \t{}", out_pretty, e); let _ = writeln!(error, "sccache: caused by: {}", e); } - stats.cache_errors.increment(&kind); + stats.cache_errors.increment(&kind, &lang); //TODO: figure out a better way to communicate this? res.retcode = Some(-2); res.stderr = error.into_bytes(); @@ -1353,9 +1354,9 @@ pub struct PerLanguageCount { } impl PerLanguageCount { - fn increment(&mut self, kind: &CompilerKind) { - let key = kind.lang_kind(); - let count = self.counts.entry(key).or_insert(0); + fn increment(&mut self, kind: &CompilerKind, lang: &Language) { + let lang_key = kind.lang_kind(lang); + let count = self.counts.entry(lang_key).or_insert(0); *count += 1; } diff --git a/tests/system.rs b/tests/system.rs index e1cfbee266..2f5e468c9d 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -98,6 +98,7 @@ const INPUT_MACRO_EXPANSION: &str = "test_macro_expansion.c"; const INPUT_WITH_DEFINE: &str = "test_with_define.c"; const INPUT_FOR_CUDA_A: &str = "test_a.cu"; const INPUT_FOR_CUDA_B: &str = "test_b.cu"; +const INPUT_FOR_CUDA_C: &str = "test_c.cu"; const OUTPUT: &str = "test.o"; // Copy the source files into the tempdir so we can compile with relative paths, since the commandline winds up in the hash key. @@ -452,7 +453,7 @@ fn run_sccache_command_tests(compiler: Compiler, tempdir: &Path) { } } -fn test_cuda_compiles(compiler: Compiler, tempdir: &Path) { +fn test_cuda_compiles(compiler: &Compiler, tempdir: &Path) { let Compiler { name, exe, @@ -521,7 +522,7 @@ fn test_cuda_compiles(compiler: Compiler, tempdir: &Path) { Vec::new(), )) .current_dir(tempdir) - .envs(env_vars) + .envs(env_vars.clone()) .assert() .success(); assert!(fs::metadata(&out_file).map(|m| m.len() > 0).unwrap()); @@ -536,8 +537,80 @@ fn test_cuda_compiles(compiler: Compiler, tempdir: &Path) { }); } +fn test_proper_lang_stat_tracking(compiler: Compiler, tempdir: &Path) { + let Compiler { + name, + exe, + env_vars, + } = compiler; + zero_stats(); + + trace!("run_sccache_command_test: {}", name); + // Compile multiple source files. + copy_to_tempdir(&[INPUT_FOR_CUDA_C, INPUT], tempdir); + + let out_file = tempdir.join(OUTPUT); + trace!("compile CUDA A"); + sccache_command() + .args(&compile_cmdline( + name, + &exe, + INPUT_FOR_CUDA_C, + OUTPUT, + Vec::new(), + )) + .current_dir(tempdir) + .envs(env_vars.clone()) + .assert() + .success(); + fs::remove_file(&out_file).unwrap(); + trace!("compile CUDA A"); + sccache_command() + .args(&compile_cmdline( + name, + &exe, + INPUT_FOR_CUDA_C, + OUTPUT, + Vec::new(), + )) + .current_dir(tempdir) + .envs(env_vars.clone()) + .assert() + .success(); + fs::remove_file(&out_file).unwrap(); + trace!("compile C++ A"); + sccache_command() + .args(&compile_cmdline(name, &exe, INPUT, OUTPUT, Vec::new())) + .current_dir(tempdir) + .envs(env_vars.clone()) + .assert() + .success(); + fs::remove_file(&out_file).unwrap(); + trace!("compile C++ A"); + sccache_command() + .args(&compile_cmdline(name, &exe, INPUT, OUTPUT, Vec::new())) + .current_dir(tempdir) + .envs(env_vars.clone()) + .assert() + .success(); + fs::remove_file(&out_file).unwrap(); + + trace!("request stats"); + get_stats(|info| { + assert_eq!(4, info.stats.compile_requests); + assert_eq!(4, info.stats.requests_executed); + assert_eq!(2, info.stats.cache_hits.all()); + assert_eq!(2, info.stats.cache_misses.all()); + assert_eq!(&1, info.stats.cache_hits.get("C/C++").unwrap()); + assert_eq!(&1, info.stats.cache_misses.get("C/C++").unwrap()); + assert_eq!(&1, info.stats.cache_hits.get("CUDA").unwrap()); + assert_eq!(&1, info.stats.cache_misses.get("CUDA").unwrap()); + }); +} + fn run_sccache_cuda_command_tests(compiler: Compiler, tempdir: &Path) { - test_cuda_compiles(compiler, tempdir); + test_cuda_compiles(&compiler, tempdir); + test_proper_lang_stat_tracking(compiler, tempdir); } fn test_clang_multicall(compiler: Compiler, tempdir: &Path) { diff --git a/tests/test_c.cu b/tests/test_c.cu new file mode 100644 index 0000000000..c6d61ec3d5 --- /dev/null +++ b/tests/test_c.cu @@ -0,0 +1,10 @@ + +#include +#include "cuda_runtime.h" + +__global__ void cuda_entry_point(int*, int*) {} +__device__ void cuda_device_func(int*, int*) {} + +int main() { + printf("%s says hello world\n", __FILE__); +}