From 801eab9ab4b801fb4ee65438cf2fda77b3a332d2 Mon Sep 17 00:00:00 2001 From: server2 Date: Tue, 20 Oct 2026 14:46:08 +0200 Subject: [PATCH] Config: each accessor caches its value per thread until the settings change A read was a lock, a hash of the path and a conversion (a colour parsed from its text), 160 to 570 ns in the unoptimised shell, a few hundred a frame with a card open. Now an atomic load and a copy while nothing changed. Every write to the settings goes through change(), which bumps the generation the caches compare against --- src/config.rs | 109 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 89 insertions(+), 20 deletions(-) diff --git a/src/config.rs b/src/config.rs index adceb8c..85e62db 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,7 +1,10 @@ use std::collections::HashMap; use std::fs; use std::path::PathBuf; +use std::cell::RefCell; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{LazyLock, Mutex, PoisonError, RwLock}; +use std::thread::LocalKey; use amane::Color; @@ -181,7 +184,11 @@ macro_rules! settings { $( #[allow(dead_code)] pub fn $name() -> $ty { - super::super::get(concat!(stringify!($module), ".", stringify!($group), ".", stringify!($name)), || $default) + std::thread_local! { + static SLOT: std::cell::RefCell<(u64, Option<$ty>)> = const { std::cell::RefCell::new((0, None)) }; + } + + super::super::cached(&SLOT, concat!(stringify!($module), ".", stringify!($group), ".", stringify!($name)), || $default) } )* } )* @@ -504,9 +511,12 @@ settings! { } } -// what config.toml set, by path; a missing or bad setting takes its default +// what config.toml set, by path; a missing or bad setting takes its default. written only through change() static VALUES: LazyLock>> = LazyLock::new(Default::default); +// how many times VALUES changed; an accessor's cached answer from an older count is read again +static GENERATION: AtomicU64 = AtomicU64::new(1); + // what was wrong in the file when it was last read, for the config window static ERRORS: Mutex> = Mutex::new(Vec::new()); @@ -521,11 +531,43 @@ fn written() -> std::sync::MutexGuard<'static, String> { WRITTEN.lock().unwrap_or_else(PoisonError::into_inner) } -// ponytail: a lock and a hash lookup per call, a few hundred a frame; a snapshot struct if a profile ever shows it fn get(path: &str, default: impl FnOnce() -> T) -> T { VALUES.read().unwrap_or_else(PoisonError::into_inner).get(path).and_then(T::read).unwrap_or_else(default) } +// an accessor's last answer on this thread, with the GENERATION it was read at +type Slot = LocalKey)>>; + +/* + * what every accessor (settings!) returns: its last answer while VALUES hasn't changed + * since, else a fresh get(). a few hundred reads a frame with a card open, each a lock, + * a hash of the path and a conversion (a colour parsed from its text) at 160 to 570 ns + * unoptimised; a cached one is an atomic load and a copy, 13 ns. per thread, so it takes + * no lock. a read racing a change may return the old value once, as get() could + */ +fn cached(slot: &'static Slot, path: &str, default: impl FnOnce() -> T) -> T { + let now = GENERATION.load(Ordering::Acquire); + + if let Some(value) = slot.with_borrow(|(seen, value)| value.clone().filter(|_| *seen == now)) { + return value; + } + + let value = get(path, default); + + slot.set((now, Some(value.clone()))); + + value +} + +// every change to VALUES: made, then counted (the write lock is gone by then), so the accessors read again +fn change(edit: impl FnOnce(&mut HashMap) -> R) -> R { + let done = edit(&mut VALUES.write().unwrap_or_else(PoisonError::into_inner)); + + GENERATION.fetch_add(1, Ordering::Release); + + done +} + pub fn file() -> PathBuf { theme::config_file("config.toml") } @@ -572,23 +614,24 @@ pub fn reread() -> bool { // shell.conf's four settings, moved to their places in config.toml fn migrate() { let old = fs::read_to_string(theme::config_file("shell.conf")).unwrap_or_default(); - let mut values = VALUES.write().unwrap_or_else(PoisonError::into_inner); - for (key, text) in parse::settings(&old) { - let path = match key.as_str() { - "cover_colors" => "appearance.colors.cover_colors", - "transition_ms" => "appearance.colors.transition_ms", - "hotspot" => "control.services.hotspot", - "extra_output" => "control.services.extra_output", - _ => continue, - }; + change(|values| { + for (key, text) in parse::settings(&old) { + let path = match key.as_str() { + "cover_colors" => "appearance.colors.cover_colors", + "transition_ms" => "appearance.colors.transition_ms", + "hotspot" => "control.services.hotspot", + "extra_output" => "control.services.extra_output", + _ => continue, + }; - if let Some(option) = option(path) - && let Ok(value) = from_text(option, &text) - { - values.insert(path.to_string(), value); + if let Some(option) = option(path) + && let Ok(value) = from_text(option, &text) + { + values.insert(path.to_string(), value); + } } - } + }); } // the file's text, read anew: what it sets replaces everything set before @@ -605,7 +648,7 @@ pub fn apply_text(text: &str) { } } - *VALUES.write().unwrap_or_else(PoisonError::into_inner) = values; + change(|old| *old = values); *ERRORS.lock().unwrap_or_else(PoisonError::into_inner) = errors; hypr::sync_soon(); @@ -705,7 +748,7 @@ pub fn set(path: &str, value: Value) -> Result<(), String> { let mut written = written(); - VALUES.write().unwrap_or_else(PoisonError::into_inner).insert(path.to_string(), value); + change(|values| values.insert(path.to_string(), value)); save(&mut written) } @@ -714,7 +757,7 @@ pub fn set(path: &str, value: Value) -> Result<(), String> { pub fn reset(path: &str) -> Result<(), String> { let mut written = written(); - VALUES.write().unwrap_or_else(PoisonError::into_inner).remove(path); + change(|values| values.remove(path)); save(&mut written) } @@ -892,3 +935,29 @@ pub fn label(name: &str) -> String { title(name) } + +#[cfg(test)] +mod tests { + use super::*; + + // the cached accessors follow every change, on this thread and on others + #[test] + fn accessors_follow_changes() { + let (width, family, accent) = (island::pill::width(), appearance::font::family(), appearance::fallback::accent()); + + change(|values| { + values.insert("island.pill.width".into(), Value::Float(321.0)); + values.insert("appearance.font.family".into(), Value::Text("Iosevka".into())); + values.insert("appearance.fallback.accent".into(), Value::Text("#102030".into())); + }); + + assert_eq!(island::pill::width(), 321.0); + assert_eq!(appearance::font::family(), "Iosevka"); + assert_eq!(appearance::fallback::accent(), Color::rgb(16, 32, 48)); + assert_eq!(std::thread::spawn(island::pill::width).join().unwrap(), 321.0); + + change(|values| values.clear()); + + assert_eq!((island::pill::width(), appearance::font::family(), appearance::fallback::accent()), (width, family, accent)); + } +}