fix: memoize quirk glob patterns and isolate concurrent override writes
This commit is contained in:
2 files changed
+29
-9
No files matched your search
+17
-2
@@ -9,7 +9,7 @@ use log::warn;
|
|||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
use std::collections::{HashMap, HashSet};
|
use std::collections::{HashMap, HashSet};
|
||||||
use std::sync::LazyLock;
|
use std::sync::{Arc, LazyLock, Mutex};
|
||||||
|
|
||||||
pub const QUIRKS_YAML: &str = include_str!("../../quirks.yaml");
|
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<Mutex<HashMap<String, Arc<Regex>>>> =
|
||||||
|
LazyLock::new(|| Mutex::new(HashMap::new()));
|
||||||
|
|
||||||
fn glob_matches(pattern: &str, name: &str) -> bool {
|
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)
|
/// Translates a glob pattern (`*` = any run of characters, `?` = exactly one)
|
||||||
|
|||||||
+12
-7
@@ -621,15 +621,20 @@ fn write_models_override(list: Vec<ProviderModels>) -> Result<PathBuf> {
|
|||||||
|
|
||||||
let model_override_path = paths::models_override_file();
|
let model_override_path = paths::models_override_file();
|
||||||
ensure_parent_exists(&model_override_path)?;
|
ensure_parent_exists(&model_override_path)?;
|
||||||
// Write through a sibling .tmp file and rename it over the target so an
|
// Write through a sibling temp file and rename it over the target so an
|
||||||
// interrupted write can never leave a truncated override behind.
|
// 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();
|
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);
|
let tmp_path = PathBuf::from(tmp_name);
|
||||||
std::fs::write(&tmp_path, models_override_data)
|
std::fs::write(&tmp_path, models_override_data)
|
||||||
.with_context(|| format!("Failed to write to '{}'", tmp_path.display()))?;
|
.with_context(|| format!("Failed to write to '{}'", tmp_path.display()))?;
|
||||||
std::fs::rename(&tmp_path, &model_override_path)
|
if let Err(error) = std::fs::rename(&tmp_path, &model_override_path) {
|
||||||
.with_context(|| format!("Failed to write to '{}'", model_override_path.display()))?;
|
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)
|
Ok(model_override_path)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1589,12 +1594,12 @@ clients:
|
|||||||
fs::write(&path, serde_yaml::to_string(&mismatched).unwrap()).unwrap();
|
fs::write(&path, serde_yaml::to_string(&mismatched).unwrap()).unwrap();
|
||||||
assert!(paths::local_models_override().is_err());
|
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.
|
// leaves a loadable override behind.
|
||||||
let written = write_models_override(vec![entry()]).unwrap();
|
let written = write_models_override(vec![entry()]).unwrap();
|
||||||
assert_eq!(written, path);
|
assert_eq!(written, path);
|
||||||
let mut tmp_name = path.as_os_str().to_os_string();
|
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());
|
assert!(!PathBuf::from(tmp_name).exists());
|
||||||
let loaded = paths::local_models_override().unwrap();
|
let loaded = paths::local_models_override().unwrap();
|
||||||
assert_eq!(loaded.len(), 1);
|
assert_eq!(loaded.len(), 1);
|
||||||
|
|||||||
Reference in new issue
Block a user