Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cache hit percentage to stats #2211

Merged
merged 5 commits into from
Jul 18, 2024
Merged

Add cache hit percentage to stats #2211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f15229
Add cache hit percentage to stats
Saruniks Jun 21, 2024
665075a
Change macro to a function
Saruniks Jun 28, 2024
f5b64f1
Add tests
Saruniks Jun 29, 2024
dd35cfa
Add integration tests
Saruniks Jul 17, 2024
1234cef
Fix undeclared types
Saruniks Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
215 changes: 200 additions & 15 deletions src/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use futures::{future, stream, Sink, SinkExt, Stream, StreamExt, TryFutureExt};
use number_prefix::NumberPrefix;
use serde::{Deserialize, Serialize};
use std::cell::Cell;
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::env;
use std::ffi::{OsStr, OsString};
use std::future::Future;
Expand Down Expand Up @@ -1510,11 +1510,23 @@ impl Default for ServerStats {
}
}

pub trait ServerStatsWriter {
fn write(&mut self, text: &str);
}

pub struct StdoutServerStatsWriter;

impl ServerStatsWriter for StdoutServerStatsWriter {
fn write(&mut self, text: &str) {
println!("{text}");
}
}

impl ServerStats {
/// Print stats to stdout in a human-readable format.
/// Print stats in a human-readable format.
///
/// Return the formatted width of each of the (name, value) columns.
fn print(&self, advanced: bool) -> (usize, usize) {
fn print<T: ServerStatsWriter>(&self, writer: &mut T, advanced: bool) -> (usize, usize) {
macro_rules! set_stat {
($vec:ident, $var:expr, $name:expr) => {{
// name, value, suffix length
Expand Down Expand Up @@ -1570,6 +1582,9 @@ impl ServerStats {
set_lang_stat!(stats_vec, self.cache_hits, "Cache hits");
set_lang_stat!(stats_vec, self.cache_misses, "Cache misses");
}

self.set_percentage_stats(&mut stats_vec, advanced);

set_stat!(stats_vec, self.cache_timeouts, "Cache timeouts");
set_stat!(stats_vec, self.cache_read_errors, "Cache read errors");
set_stat!(stats_vec, self.forced_recaches, "Forced recaches");
Expand Down Expand Up @@ -1627,45 +1642,110 @@ impl ServerStats {
let name_width = stats_vec.iter().map(|(n, _, _)| n.len()).max().unwrap();
let stat_width = stats_vec.iter().map(|(_, s, _)| s.len()).max().unwrap();
for (name, stat, suffix_len) in stats_vec {
println!(
writer.write(&format!(
"{:<name_width$} {:>stat_width$}",
name,
stat,
name_width = name_width,
stat_width = stat_width + suffix_len
);
));
}
if !self.dist_compiles.is_empty() {
println!("\nSuccessful distributed compiles");
writer.write("\nSuccessful distributed compiles");
let mut counts: Vec<_> = self.dist_compiles.iter().collect();
counts.sort_by(|(_, c1), (_, c2)| c1.cmp(c2).reverse());
for (reason, count) in counts {
println!(
writer.write(&format!(
" {:<name_width$} {:>stat_width$}",
reason,
count,
name_width = name_width - 2,
stat_width = stat_width
);
stat_width = stat_width,
));
}
}
if !self.not_cached.is_empty() {
println!("\nNon-cacheable reasons:");
writer.write("\nNon-cacheable reasons:");
let mut counts: Vec<_> = self.not_cached.iter().collect();
counts.sort_by(|(_, c1), (_, c2)| c1.cmp(c2).reverse());
for (reason, count) in counts {
println!(
writer.write(&format!(
"{:<name_width$} {:>stat_width$}",
reason,
count,
name_width = name_width,
stat_width = stat_width
);
stat_width = stat_width,
));
}
println!();
writer.write("");
}
(name_width, stat_width)
}

fn set_percentage_stats(&self, stats_vec: &mut Vec<(String, String, usize)>, advanced: bool) {
set_percentage_stat(
stats_vec,
self.cache_hits.all(),
self.cache_misses.all() + self.cache_hits.all(),
"Cache hits rate",
);

let (stats_hits, stats_misses): (Vec<_>, Vec<_>) = if advanced {
(
self.cache_hits.adv_counts.iter().collect(),
self.cache_misses.adv_counts.iter().collect(),
)
} else {
(
self.cache_hits.counts.iter().collect(),
self.cache_misses.counts.iter().collect(),
)
};

let mut all_languages: HashSet<&String> = HashSet::new();
for (lang, _) in &stats_hits {
all_languages.insert(lang);
}
for (lang, _) in &stats_misses {
all_languages.insert(lang);
}

let mut all_languages: Vec<&String> = all_languages.into_iter().collect();
all_languages.sort();

for lang in all_languages {
let count_hits = stats_hits
.iter()
.find(|&&(l, _)| l == lang)
.map_or(0, |&(_, &count)| count);

let count_misses = stats_misses
.iter()
.find(|&&(l, _)| l == lang)
.map_or(0, |&(_, &count)| count);

set_percentage_stat(
stats_vec,
count_hits,
count_hits + count_misses,
&format!("Cache hits rate ({})", lang),
);
}
}
}

fn set_percentage_stat(
vec: &mut Vec<(String, String, usize)>,
count_hits: u64,
total: u64,
name: &str,
) {
if total == 0 {
vec.push((name.to_string(), "-".to_string(), 0));
} else {
let ratio = count_hits as f64 / total as f64;
vec.push((name.to_string(), format!("{:.2} %", ratio * 100.0), 2));
}
}

impl ServerInfo {
Expand Down Expand Up @@ -1700,7 +1780,7 @@ impl ServerInfo {

/// Print info to stdout in a human-readable format.
pub fn print(&self, advanced: bool) {
let (name_width, stat_width) = self.stats.print(advanced);
let (name_width, stat_width) = self.stats.print(&mut StdoutServerStatsWriter, advanced);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intrusive way to test the code. If anyone has an idea on how to test it in a better way it would be helpful to know.

I was thinking about writing similar test like in sccache_cargo.rs (using Command::new() and capturing the output), but I was hesitant because the test would not be fast.

I was thinking about refactoring this code to free functions and testing the free functions but then it would potentially not cover the whole use case.

println!(
"{:<name_width$} {}",
"Cache location",
Expand Down Expand Up @@ -1978,3 +2058,108 @@ fn waits_until_zero() {
drop(active2);
assert_eq!(wait.now_or_never(), Some(()));
}

#[cfg(test)]
mod tests {
use super::*;

struct StringWriter {
buffer: String,
}

impl StringWriter {
fn new() -> StringWriter {
StringWriter {
buffer: String::new(),
}
}

fn get_output(self) -> String {
self.buffer
}
}

impl ServerStatsWriter for StringWriter {
fn write(&mut self, text: &str) {
self.buffer.push_str(&format!("{}\n", text));
}
}

#[test]
fn test_print_cache_hits_rate_default_server_stats() {
let stats = ServerStats::default();

let mut writer = StringWriter::new();
stats.print(&mut writer, false);

let output = writer.get_output();

assert!(output.contains("Cache hits rate -"));
}

#[test]
fn test_print_cache_hits_rate_server_stats() {
let mut cache_hits_counts = HashMap::new();
cache_hits_counts.insert("Rust".to_string(), 100);
cache_hits_counts.insert("C/C++".to_string(), 200);

let mut cache_misses_counts = HashMap::new();
cache_misses_counts.insert("Rust".to_string(), 50);
cache_misses_counts.insert("Cuda".to_string(), 300);

let stats = ServerStats {
cache_hits: PerLanguageCount {
counts: cache_hits_counts,
..Default::default()
},
cache_misses: PerLanguageCount {
counts: cache_misses_counts,
..Default::default()
},
..Default::default()
};

let mut writer = StringWriter::new();
stats.print(&mut writer, false);

let output = writer.get_output();

assert!(output.contains("Cache hits rate 46.15 %"));
assert!(output.contains("Cache hits rate (C/C++) 100.00 %"));
assert!(output.contains("Cache hits rate (Cuda) 0.00 %"));
assert!(output.contains("Cache hits rate (Rust) 66.67 %"));
}

#[test]
fn test_print_cache_hits_rate_advanced_server_stats() {
let mut cache_hits_counts = HashMap::new();
cache_hits_counts.insert("rust".to_string(), 50);
cache_hits_counts.insert("c/c++ [clang]".to_string(), 30);

let mut cache_misses_counts = HashMap::new();
cache_misses_counts.insert("rust".to_string(), 100);
cache_misses_counts.insert("cuda".to_string(), 70);

let stats = ServerStats {
cache_hits: PerLanguageCount {
adv_counts: cache_hits_counts,
..Default::default()
},
cache_misses: PerLanguageCount {
adv_counts: cache_misses_counts,
..Default::default()
},
..Default::default()
};

let mut writer = StringWriter::new();
stats.print(&mut writer, true);

let output = writer.get_output();

assert!(output.contains("Cache hits rate -"));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting case where PerLanguageCount::all() uses counts.values().sum() and not self.adv_counts.values().sum() in case of printing advanced stats.

In practice should never happen because PerLanguageCount::increment() is used to set the counts.

assert!(output.contains("Cache hits rate (c/c++ [clang]) 100.00 %"));
assert!(output.contains("Cache hits rate (cuda) 0.00 %"));
assert!(output.contains("Cache hits rate (rust) 33.33 %"));
}
}
Loading
Loading