Merge branch 'config-cache'

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)
                     }
                 )*
             } )*
@@ -615,9 +622,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<RwLock<HashMap<String, Value>>> = 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<Vec<String>> = Mutex::new(Vec::new());
 
@@ -632,11 +642,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<T: Setting>(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<T> = LocalKey<RefCell<(u64, Option<T>)>>;
+
+/*
+ * 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<T: Setting + Clone>(slot: &'static Slot<T>, 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<R>(edit: impl FnOnce(&mut HashMap<String, Value>) -> 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")
 }
@@ -683,23 +725,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) {
+    change(|values| {
-        let path = match key.as_str() {
+        for (key, text) in parse::settings(&old) {
-            "cover_colors" => "appearance.colors.cover_colors",
+            let path = match key.as_str() {
-            "transition_ms" => "appearance.colors.transition_ms",
+                "cover_colors" => "appearance.colors.cover_colors",
-            "hotspot" => "control.services.hotspot",
+                "transition_ms" => "appearance.colors.transition_ms",
-            "extra_output" => "control.services.extra_output",
+                "hotspot" => "control.services.hotspot",
-            _ => continue,
+                "extra_output" => "control.services.extra_output",
-        };
+                _ => continue,
+            };
 
-        if let Some(option) = option(path)
+            if let Some(option) = option(path)
-            && let Ok(value) = from_text(option, &text)
+                && let Ok(value) = from_text(option, &text)
-        {
+            {
-            values.insert(path.to_string(), value);
+                values.insert(path.to_string(), value);
+            }
         }
-    }
+    });
 }
 
 // the file's text, read anew: what it sets replaces everything set before
@@ -716,7 +759,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();
@@ -816,7 +859,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)
 }
@@ -825,7 +868,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)
 }
@@ -1004,3 +1047,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));
+    }
+}