From 47d2edf303d174fb5fb1b1a00149c02a4bfbcb7e Mon Sep 17 00:00:00 2001 From: Alex Clarke Date: Wed, 2 Sep 2026 16:02:41 -0600 Subject: [PATCH] fix: memoize quirk glob patterns and isolate concurrent override writes --- src/client/catalog.rs | 19 +++++++++++++++++-- src/config/mod.rs | 19 ++++++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/client/catalog.rs b/src/client/catalog.rs index e1b3aa5..9742849 100644 --- a/src/client/catalog.rs +++ b/src/client/catalog.rs @@ -9,7 +9,7 @@ use log::warn; use serde::{Deserialize, Serialize}; use serde_json::Value; use std::collections::{HashMap, HashSet}; -use std::sync::LazyLock; +use std::sync::{Arc, LazyLock, Mutex}; pub const QUIRKS_YAML: &str = include_str!("../../quirks.yaml"); @@ -815,8 +815,23 @@ pub fn quirks_for<'a>( }) } +/// Compiled patterns are memoized: catalog builds evaluate every rule, exclude, +/// and variant glob against every model, so recompiling per call would dominate +/// the merge as the catalog grows. +static GLOB_CACHE: LazyLock>>> = + LazyLock::new(|| Mutex::new(HashMap::new())); + fn glob_matches(pattern: &str, name: &str) -> bool { - compile_glob(pattern).is_match(name).unwrap_or(false) + let regex = { + let mut cache = GLOB_CACHE + .lock() + .unwrap_or_else(|poison| poison.into_inner()); + cache + .entry(pattern.to_string()) + .or_insert_with(|| Arc::new(compile_glob(pattern))) + .clone() + }; + regex.is_match(name).unwrap_or(false) } /// Translates a glob pattern (`*` = any run of characters, `?` = exactly one) diff --git a/src/config/mod.rs b/src/config/mod.rs index 372c59b..2873817 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -621,15 +621,20 @@ fn write_models_override(list: Vec) -> Result { let model_override_path = paths::models_override_file(); ensure_parent_exists(&model_override_path)?; - // Write through a sibling .tmp file and rename it over the target so an - // interrupted write can never leave a truncated override behind. + // Write through a sibling temp file and rename it over the target so an + // interrupted write can never leave a truncated override behind. The temp + // name embeds the pid so concurrent coyote processes (auto-refresh racing + // a manual --sync-models) never interleave writes into the same file. let mut tmp_name = model_override_path.as_os_str().to_os_string(); - tmp_name.push(".tmp"); + tmp_name.push(format!(".{}.tmp", std::process::id())); let tmp_path = PathBuf::from(tmp_name); std::fs::write(&tmp_path, models_override_data) .with_context(|| format!("Failed to write to '{}'", tmp_path.display()))?; - std::fs::rename(&tmp_path, &model_override_path) - .with_context(|| format!("Failed to write to '{}'", model_override_path.display()))?; + if let Err(error) = std::fs::rename(&tmp_path, &model_override_path) { + let _ = std::fs::remove_file(&tmp_path); + return Err(error) + .with_context(|| format!("Failed to write to '{}'", model_override_path.display())); + } Ok(model_override_path) } @@ -1589,12 +1594,12 @@ clients: fs::write(&path, serde_yaml::to_string(&mismatched).unwrap()).unwrap(); assert!(paths::local_models_override().is_err()); - // write_models_override goes through the sibling .tmp file and + // write_models_override goes through the sibling temp file and // leaves a loadable override behind. let written = write_models_override(vec![entry()]).unwrap(); assert_eq!(written, path); let mut tmp_name = path.as_os_str().to_os_string(); - tmp_name.push(".tmp"); + tmp_name.push(format!(".{}.tmp", std::process::id())); assert!(!PathBuf::from(tmp_name).exists()); let loaded = paths::local_models_override().unwrap(); assert_eq!(loaded.len(), 1);