From 2c044de38f6adcfbd3cc64eb9fa55e67d61e6f94 Mon Sep 17 00:00:00 2001 From: salmonik Date: Mon, 2 Nov 2026 13:22:44 +0100 Subject: config: parse the config once and reuse it until the file changes Every command, status line and notification went through Config::load, which reads *and* rewrites config.toml, so `rsp status` alone touched the file on each call. The daemon now keeps a parsed copy and only stats the file to notice outside edits, so `rsp settings set` still takes effect immediately. Config::save refreshes the cached copy too, and Config::config_path is shared by the loader and the cache. --- src/config.rs | 107 ++++++++++++++++++++++++++++++++++++++++++++++++---------- src/daemon.rs | 54 +++++++++++++++-------------- src/mpris.rs | 12 +++---- 3 files changed, 124 insertions(+), 49 deletions(-) diff --git a/src/config.rs b/src/config.rs index f20fb3a..a08f01f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -9,8 +9,10 @@ use directories::ProjectDirs; use serde::{Deserialize, Serialize}; use std::fs; use std::path::PathBuf; +use std::sync::{LazyLock, RwLock}; +use std::time::SystemTime; -#[derive(Debug, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct Config { #[serde(skip)] pub config_path: PathBuf, @@ -25,24 +27,29 @@ pub struct Config { pub display_format: Option, } +/// Identity of the config file on disk, used to tell whether a cached copy is +/// still current. `modified` alone is not enough: several filesystems only +/// store whole seconds, so a quick edit could keep the same timestamp. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct FileStamp { + modified: Option, + len: u64, +} + +struct CachedConfig { + config: Config, + stamp: FileStamp, +} + +static CACHE: LazyLock>> = LazyLock::new(|| RwLock::new(None)); + impl Config { pub fn load() -> Result> { - let config_dir = if cfg!(debug_assertions) { - std::env::current_exe()? - .parent() - .map(|p| p.to_path_buf()) - .unwrap_or_else(|| PathBuf::from(".")) - } else { - let proj_dirs = ProjectDirs::from("com", "rsplayer", "rsplayer") - .ok_or("Failed to get project directories")?; - if let Ok(xdg) = std::env::var("XDG_CONFIG_HOME") { - PathBuf::from(xdg).join("rsplayer") - } else { - proj_dirs.config_dir().to_path_buf() - } - }; - - let config_path = config_dir.join("config.toml"); + let config_path = Self::config_path()?; + let config_dir = config_path + .parent() + .map(|p| p.to_path_buf()) + .unwrap_or_else(|| PathBuf::from(".")); if !config_path.exists() { fs::create_dir_all(&config_dir)?; @@ -76,6 +83,64 @@ impl Config { Ok(config) } + /// Where the config lives: next to the binary for debug builds (so a + /// checkout does not touch the installed config), XDG config otherwise. + pub fn config_path() -> Result> { + if cfg!(debug_assertions) { + return Ok(std::env::current_exe()? + .parent() + .map(|p| p.to_path_buf()) + .unwrap_or_else(|| PathBuf::from(".")) + .join("config.toml")); + } + let proj_dirs = ProjectDirs::from("com", "rsplayer", "rsplayer") + .ok_or("Failed to get project directories")?; + let config_dir = if let Ok(xdg) = std::env::var("XDG_CONFIG_HOME") { + PathBuf::from(xdg).join("rsplayer") + } else { + proj_dirs.config_dir().to_path_buf() + }; + Ok(config_dir.join("config.toml")) + } + + fn file_stamp() -> FileStamp { + let stamp = Self::config_path() + .ok() + .and_then(|path| fs::metadata(path).ok()); + FileStamp { + modified: stamp.as_ref().and_then(|m| m.modified().ok()), + len: stamp.as_ref().map(|m| m.len()).unwrap_or(0), + } + } + + /// The config, parsed once and reused until the file changes on disk. + /// + /// The daemon reads the config for every command, status line and + /// notification, and [`Config::load`] parses the TOML *and* rewrites the + /// file. Checking a stat instead keeps that off the hot paths while + /// `rsp settings set` still takes effect immediately. + pub fn cached() -> Result> { + let stamp = Self::file_stamp(); + if let Ok(guard) = CACHE.read() + && let Some(cached) = guard.as_ref() + && cached.stamp == stamp + { + return Ok(cached.config.clone()); + } + + let config = Self::load()?; + // `load` normalizes and rewrites the file, so take the stamp afterwards: + // reading it before would make our own write look like an outside edit. + let stamp = Self::file_stamp(); + if let Ok(mut guard) = CACHE.write() { + *guard = Some(CachedConfig { + config: config.clone(), + stamp, + }); + } + Ok(config) + } + pub fn save(&self) -> Result<(), Box> { let mut output = String::new(); let paths_str = self @@ -131,6 +196,14 @@ impl Config { fs::create_dir_all(parent)?; } fs::write(&self.config_path, output)?; + // Keep the parsed cache in step with what was just written, so this + // write does not make the next `cached()` re-read the file. + if let Ok(mut guard) = CACHE.write() + && let Some(cached) = guard.as_mut() + { + cached.stamp = Self::file_stamp(); + cached.config = self.clone(); + } Ok(()) } diff --git a/src/daemon.rs b/src/daemon.rs index 4f95820..eaae64f 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -190,7 +190,7 @@ pub fn spawn_try_scrobble_current( } pub fn start_daemon() -> Result<(), Box> { - let config = Config::load()?; + let config = Config::cached()?; let socket_path = Config::get_socket_path(); let pid_path = get_pid_path(&config); @@ -365,7 +365,7 @@ pub fn start_daemon() -> Result<(), Box> { if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(&track, fmt.as_deref()); break; } @@ -448,7 +448,7 @@ pub fn start_daemon() -> Result<(), Box> { } pub fn stop_daemon() -> Result<(), Box> { - let config = Config::load()?; + let config = Config::cached()?; let pid_path = get_pid_path(&config); match check_running_daemon(&pid_path) { @@ -708,7 +708,7 @@ enum LoadedLyrics { /// Build the "Now playing" header shown above a track's lyrics. fn lyrics_header(track: &Track) -> String { - let name = match Config::load() + let name = match Config::cached() .ok() .and_then(|c| c.display_format.clone()) .as_deref() @@ -1022,7 +1022,7 @@ fn process_command( "Paused" }; let track = Track::from_path(track_path); - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); let track_str = match fmt.as_deref() { Some(f) => track.format(f), None => track.display_string(0, 1), @@ -1094,7 +1094,7 @@ fn process_command( "Shutting down\n".to_string() } "reload" | "validate" => { - if let Ok(config) = Config::load() { + if let Ok(config) = Config::cached() { let temp_scrobbler = Scrobbler::new( config.lastfm_api_key.clone(), config.lastfm_api_secret.clone(), @@ -1137,7 +1137,7 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(&track, fmt.as_deref()); "Playing next track\n".to_string() } else { @@ -1160,7 +1160,7 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(&track, fmt.as_deref()); "Playing previous track\n".to_string() } else { @@ -1202,7 +1202,7 @@ fn process_command( "Usage: shuffle [on|off|album|albums]\n".to_string() }; // Save to config - if let Ok(mut config) = Config::load() { + if let Ok(mut config) = Config::cached() { if mode_str == "off" || mode_str == "none" { config.shuffle = None; } else { @@ -1242,7 +1242,7 @@ fn process_command( let (album_tracks, display_name): (Vec, String) = if path.is_file() { let canon = std::fs::canonicalize(&path).unwrap_or_else(|_| path.clone()); let (artist, album, _, _, _) = crate::database::extract_metadata(&canon); - let full: Vec = match Config::load() { + let full: Vec = match Config::cached() { Ok(config) => match crate::database::Database::open(&config) { Ok(db) => db .find_tracks_by_album(&artist, &album) @@ -1267,7 +1267,7 @@ fn process_command( .to_string(); (rotated, name) } else { - match Config::load() { + match Config::cached() { Ok(config) => match crate::database::Database::open(&config) { Ok(db) => { let tracks = match db.resolve_path(query_str) { @@ -1352,7 +1352,7 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(track, fmt.as_deref()); } format!( @@ -1464,7 +1464,7 @@ fn process_command( s.start(track.clone()); } let fmt = - Config::load().ok().and_then(|c| c.display_format.clone()); + Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(track, fmt.as_deref()); } format!("Playing now: {}\n", path_str) @@ -1486,7 +1486,7 @@ fn process_command( s.start(track.clone()); } let fmt = - Config::load().ok().and_then(|c| c.display_format.clone()); + Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(track, fmt.as_deref()); } format!("Queued: {}\n", path_str) @@ -1514,8 +1514,9 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = - Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached() + .ok() + .and_then(|c| c.display_format.clone()); notification::notify_track(track, fmt.as_deref()); } format!("Playing now: {}\n", path_str) @@ -1535,8 +1536,9 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = - Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached() + .ok() + .and_then(|c| c.display_format.clone()); notification::notify_track(&track, fmt.as_deref()); format!("Playing: {}\n", path_str) } else { @@ -1568,7 +1570,7 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load() + let fmt = Config::cached() .ok() .and_then(|c| c.display_format.clone()); notification::notify_track(track, fmt.as_deref()); @@ -1602,7 +1604,7 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load() + let fmt = Config::cached() .ok() .and_then(|c| c.display_format.clone()); notification::notify_track(track, fmt.as_deref()); @@ -1622,7 +1624,7 @@ fn process_command( _ => format!("Error: No audio files found in '{}'\n", path_str), } } else { - match Config::load() { + match Config::cached() { Ok(config) => match crate::database::Database::open(&config) { Ok(db) => { // Collect tracks: try hierarchical path first, fallback to search @@ -1666,7 +1668,7 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load() + let fmt = Config::cached() .ok() .and_then(|c| c.display_format.clone()); notification::notify_track(track, fmt.as_deref()); @@ -1710,7 +1712,7 @@ fn process_command( if let Ok(mut s) = scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load() + let fmt = Config::cached() .ok() .and_then(|c| c.display_format.clone()); notification::notify_track( @@ -1772,7 +1774,7 @@ fn process_command( let current = audio.get_volume() as i8; let new_level = (current + sign * delta).clamp(0, 100) as u8; audio.set_volume(new_level as f32 / 100.0); - if let Ok(mut config) = Config::load() { + if let Ok(mut config) = Config::cached() { config.volume = Some(new_level); let _ = config.save(); } @@ -1781,7 +1783,7 @@ fn process_command( // Try absolute value if let Ok(level) = arg.parse::() { audio.set_volume(level as f32 / 100.0); - if let Ok(mut config) = Config::load() { + if let Ok(mut config) = Config::cached() { config.volume = Some(level); let _ = config.save(); } @@ -1811,7 +1813,7 @@ fn process_command( q.shuffled_indices().to_vec() }; let is_playing = audio.is_playing(); - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); for (display_idx, &track_idx) in indices.iter().enumerate() { if let Some(track) = q.track_at(track_idx) { let track_str = match fmt.as_deref() { diff --git a/src/mpris.rs b/src/mpris.rs index de19113..9daf516 100644 --- a/src/mpris.rs +++ b/src/mpris.rs @@ -204,7 +204,7 @@ impl MprisPlayer { if let Ok(mut s) = self.scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(&track, fmt.as_deref()); } else { self.audio.stop(); @@ -226,7 +226,7 @@ impl MprisPlayer { if let Ok(mut s) = self.scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(&track, fmt.as_deref()); } } @@ -271,7 +271,7 @@ impl MprisPlayer { if let Ok(mut s) = self.scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(track, fmt.as_deref()); } } @@ -319,7 +319,7 @@ impl MprisPlayer { if let Ok(mut s) = self.scrobble_state.lock() { s.start(t.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(t, fmt.as_deref()); } } else if path.is_file() { @@ -331,7 +331,7 @@ impl MprisPlayer { if let Ok(mut s) = self.scrobble_state.lock() { s.start(track.clone()); } - let fmt = Config::load().ok().and_then(|c| c.display_format.clone()); + let fmt = Config::cached().ok().and_then(|c| c.display_format.clone()); notification::notify_track(&track, fmt.as_deref()); } } @@ -417,7 +417,7 @@ impl MprisPlayer { fn set_volume(&self, value: f64) { let level = (value * 100.0).clamp(0.0, 100.0) as u8; self.audio.set_volume(level as f32 / 100.0); - if let Ok(mut config) = Config::load() { + if let Ok(mut config) = Config::cached() { config.volume = Some(level); let _ = config.save(); } -- cgit v1.2.3