Skip to content

Commit

Permalink
[crater] Cache output of fontmake between runs
Browse files Browse the repository at this point in the history
This clears the cache if the pip freeze output is different.

It's possible that this isn't worth the complexity, but it should offer
meaningful runtime speed improvements, so let's give it a try?
  • Loading branch information
cmyr committed Oct 30, 2024
1 parent 7b18241 commit edd2615
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 8 deletions.
12 changes: 10 additions & 2 deletions fontc_crater/src/ci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@ use crate::{
};

mod html;
mod results_cache;

pub(crate) use results_cache::ResultsCache;

static SUMMARY_FILE: &str = "summary.json";
static SOURCES_FILE: &str = "sources.json";

type DiffResults = Results<DiffOutput, DiffError>;

/// A summary of a single CI run

#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
struct RunSummary {
began: DateTime<Utc>,
Expand Down Expand Up @@ -103,6 +105,11 @@ fn run_crater_and_save_results(args: &CiArgs) -> Result<(), Error> {
// for now we are going to be cloning each repo freshly
let cache_dir = args.cache_dir();
log::info!("using cache dir {}", cache_dir.display());
let results_cache = ResultsCache::in_dir(&cache_dir);
if Some(&pip_freeze_sha) != prev_runs.last().map(|run| &run.pip_freeze_sha) {
log::info!("pip output has changed, clearing cached results");
results_cache.delete_all();
}

// we want to build fontc & normalizer once, and then move them out of the
// build directory so that they aren't accidentally rebuilt or deleted
Expand All @@ -119,7 +126,8 @@ fn run_crater_and_save_results(args: &CiArgs) -> Result<(), Error> {
let context = super::ttx_diff_runner::TtxContext {
fontc_path,
normalizer_path,
cache_dir,
source_cache: cache_dir,
results_cache,
};

let began = Utc::now();
Expand Down
118 changes: 118 additions & 0 deletions fontc_crater/src/ci/results_cache.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
//! Caching fontmake's output between runs

use std::{
ffi::OsStr,
path::{Path, PathBuf},
};

use crate::Target;

static CACHE_DIR_NAME: &str = "crater_cached_results";

// the files that we cache for each target
static FONT_FILE: &str = "fontmake.ttf";
static TTX_FILE: &str = "fontmake.ttx";
static MARKKERN_FILE: &str = "fontmake.markkern.txt";

/// Manages a cache of files on disk
pub(crate) struct ResultsCache {
base_results_cache_dir: PathBuf,
}

impl ResultsCache {
/// argument is the directory that will contain the cache dir.
///
/// By convention this is the same directory where we checkout git repos.
pub fn in_dir(path: &Path) -> Self {
Self {
base_results_cache_dir: path.join(CACHE_DIR_NAME),
}
}

/// Delete any cache contents
pub fn delete_all(&self) {
if self.base_results_cache_dir.exists() {
std::fs::remove_dir_all(&self.base_results_cache_dir).expect("failed to remove cache")
}
}

/// if we have cached files for this target, copy them into the build directory.
pub fn copy_cached_files_to_build_dir(&self, target: &Target, build_dir: &Path) {
let target_cache_dir = self.cache_dir_for_target(target);
if !target_cache_dir.exists() {
log::info!("{} does not exist, skipping", target_cache_dir.display());
return;
}

if copy_cache_files(&target_cache_dir, build_dir).unwrap() {
log::info!("reused cached files for {target}",);
}
}

/// Copy files generated from a previous run into the permanent cache.
pub fn save_built_files_to_cache(&self, target: &Target, build_dir: &Path) {
let target_cache_dir = self.cache_dir_for_target(target);
if !target_cache_dir.exists() {
std::fs::create_dir_all(&target_cache_dir).unwrap();
}
// no need to overwrite existing cache
if [FONT_FILE, TTX_FILE, MARKKERN_FILE]
.into_iter()
.all(|p| target_cache_dir.join(p).exists())
{
return;
}
if copy_cache_files(build_dir, &target_cache_dir).unwrap() {
log::debug!("cached files to {}", target_cache_dir.display());
}
}

/// Return the directory for storing cache results for this specific target.
///
///
/// our directory structure is:
/// BASE/{org}/{repo}/{config_stem}/{file_stem}/{build}
fn cache_dir_for_target(&self, target: &Target) -> PathBuf {
let source_stem = target.source.file_stem().unwrap();
let repo = target
.source
.parent()
.unwrap()
.parent()
.expect("sources always in format org/repo/source/FILE");
let org = repo.parent().unwrap().file_name().unwrap();
let repo = repo.file_name().unwrap();
let config = target.config.file_stem().unwrap_or(OsStr::new("config"));
let mut result = self.base_results_cache_dir.join(org);
result.push(repo);
result.extend([config, source_stem, OsStr::new(target.build.name())]);
result
}
}

fn copy_cache_files(from_dir: &Path, to_dir: &Path) -> std::io::Result<bool> {
let font = from_dir.join(FONT_FILE);
let ttx = from_dir.join(TTX_FILE);
let markkern = from_dir.join(MARKKERN_FILE);

if all_exist([&font, &ttx, &markkern]) {
if !to_dir.exists() {
std::fs::create_dir_all(to_dir)?;
}
std::fs::copy(font, to_dir.join(FONT_FILE)).map(|_| ())?;
std::fs::copy(ttx, to_dir.join(TTX_FILE)).map(|_| ())?;
std::fs::copy(markkern, to_dir.join(MARKKERN_FILE)).map(|_| ())?;
Ok(true)
} else {
Ok(false)
}
}

fn all_exist(paths: [&Path; 3]) -> bool {
for path in paths {
if !path.is_file() {
return false;
}
}
true
}
1 change: 0 additions & 1 deletion fontc_crater/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ struct Results<T, E> {

#[derive(Clone, Debug)]
pub(crate) struct Target {
// will be used in gftools mode
pub(crate) config: PathBuf,
pub(crate) source: PathBuf,
pub(crate) build: BuildType,
Expand Down
30 changes: 25 additions & 5 deletions fontc_crater/src/ttx_diff_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,39 @@ use std::{
process::Command,
};

use crate::{BuildType, Results, RunResult, Target};
use crate::{ci::ResultsCache, BuildType, Results, RunResult, Target};

static SCRIPT_PATH: &str = "./resources/scripts/ttx_diff.py";

pub(super) struct TtxContext {
pub fontc_path: PathBuf,
pub normalizer_path: PathBuf,
pub cache_dir: PathBuf,
pub source_cache: PathBuf,
pub results_cache: ResultsCache,
}

pub(super) fn run_ttx_diff(ctx: &TtxContext, target: &Target) -> RunResult<DiffOutput, DiffError> {
let tempdir = tempfile::tempdir().expect("couldn't create tempdir");
let outdir = tempdir.path();
let source_path = ctx.cache_dir.join(&target.source);
let source_path = ctx.source_cache.join(&target.source);
let compare = target.build.name();
let build_dir = outdir.join(compare);
ctx.results_cache
.copy_cached_files_to_build_dir(target, &build_dir);
let mut cmd = Command::new("python");
cmd.args([SCRIPT_PATH, "--json", "--compare", compare, "--outdir"])
.arg(outdir)
.arg("--fontc_path")
.arg(&ctx.fontc_path)
.arg("--normalizer_path")
.arg(&ctx.normalizer_path);
.arg(&ctx.normalizer_path)
.args(["--rebuild", "fontc"]);
if target.build == BuildType::GfTools {
cmd.arg("--config").arg(&target.config);
}
cmd.arg(source_path);
cmd.arg(source_path)
// set this flag so we have a stable 'modified date'
.env("SOURCE_DATE_EPOCH", "1730302089");
let output = match cmd.output() {
Err(e) => return RunResult::Fail(DiffError::Other(e.to_string())),
Ok(val) => val,
Expand Down Expand Up @@ -77,9 +84,22 @@ pub(super) fn run_ttx_diff(ctx: &TtxContext, target: &Target) -> RunResult<DiffO
// so it is useful to see them in our logs.
log::warn!("error running {target} '{err}'");
}

if fontmake_finished(&result) {
ctx.results_cache
.save_built_files_to_cache(target, &build_dir);
}
result
}

fn fontmake_finished(result: &RunResult<DiffOutput, DiffError>) -> bool {
match result {
RunResult::Success(_) => true,
RunResult::Fail(DiffError::CompileFailed(diff)) => diff.fontmake.is_none(),
RunResult::Fail(DiffError::Other(_)) => false,
}
}

#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "snake_case")]
pub(super) enum DiffOutput {
Expand Down
2 changes: 2 additions & 0 deletions resources/scripts/ttx_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ def run_gftools(
filename = tool + ".ttf"
out_file = build_dir / filename
out_dir = build_dir / "gftools_temp_dir"
if out_dir.exists():
shutil.rmtree(out_dir)
cmd = [
"gftools",
"builder",
Expand Down

0 comments on commit edd2615

Please sign in to comment.