mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 08:48:05 +00:00
perf(config): mtime-signature fast path for load_config (#410)
load_config re-read and re-parsed config.json, the secrets file, AND the template (running the recursive migration diff) on every call — with ~30 call sites in web request handlers, some hit 2-3x per request. Fast path: stat all three files (mtime_ns + size); when unchanged since the last successful load, return the already-parsed self.config (same aliasing semantics as before). The signature is taken AFTER load + migration so a migration write-back doesn't retrigger, and both save paths refresh it. Cross-process freshness is preserved by construction: a save from the other process bumps the file mtime, so the next load here re-reads — verified by a dedicated test. Same-second edits are caught by mtime_ns plus a size check. Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Chuck
Claude Fable 5
parent
6499794c12
commit
bff13129c4
+46
-4
@@ -56,6 +56,13 @@ class ConfigManager:
|
|||||||
self.secrets_path: str = secrets_path or "config/config_secrets.json"
|
self.secrets_path: str = secrets_path or "config/config_secrets.json"
|
||||||
self.template_path: str = "config/config.template.json"
|
self.template_path: str = "config/config.template.json"
|
||||||
self.config: Dict[str, Any] = {}
|
self.config: Dict[str, Any] = {}
|
||||||
|
# (mtime_ns, size) signature of (config, secrets, template) at the
|
||||||
|
# last successful load. load_config() skips the full re-read (3 file
|
||||||
|
# parses + recursive template migration) when nothing changed —
|
||||||
|
# ~30 web request handlers call it, some 2-3x per request. Cross-
|
||||||
|
# process freshness is preserved: another process's save bumps the
|
||||||
|
# mtime, so the next load here re-reads.
|
||||||
|
self._loaded_sig: Optional[tuple] = None
|
||||||
self.logger: logging.Logger = get_logger(__name__)
|
self.logger: logging.Logger = get_logger(__name__)
|
||||||
|
|
||||||
# Initialize atomic config manager
|
# Initialize atomic config manager
|
||||||
@@ -122,6 +129,14 @@ class ConfigManager:
|
|||||||
# Update in-memory config if save was successful
|
# Update in-memory config if save was successful
|
||||||
if result.status == SaveResultStatus.SUCCESS:
|
if result.status == SaveResultStatus.SUCCESS:
|
||||||
self.config = new_config_data
|
self.config = new_config_data
|
||||||
|
# In-memory config now matches what was just written; refresh
|
||||||
|
# the load signature so the fast path stays valid. NOTE: the
|
||||||
|
# in-memory copy includes merged secrets; the on-disk file has
|
||||||
|
# them stripped — the fast path returning self.config preserves
|
||||||
|
# exactly the pre-cache behavior (load-after-save also returned
|
||||||
|
# the secret-merged self.config only after re-reading secrets;
|
||||||
|
# here secrets file is unchanged, so contents are equivalent).
|
||||||
|
self._loaded_sig = self._files_signature()
|
||||||
self.logger.info(f"Configuration successfully saved atomically to {os.path.abspath(self.config_path)}")
|
self.logger.info(f"Configuration successfully saved atomically to {os.path.abspath(self.config_path)}")
|
||||||
elif result.status == SaveResultStatus.ROLLED_BACK:
|
elif result.status == SaveResultStatus.ROLLED_BACK:
|
||||||
# Reload config from file after rollback
|
# Reload config from file after rollback
|
||||||
@@ -179,13 +194,36 @@ class ConfigManager:
|
|||||||
atomic_mgr = self._get_atomic_manager()
|
atomic_mgr = self._get_atomic_manager()
|
||||||
return atomic_mgr.validate_config_file(config_path)
|
return atomic_mgr.validate_config_file(config_path)
|
||||||
|
|
||||||
|
def _files_signature(self) -> tuple:
|
||||||
|
"""(mtime_ns, size) of config/secrets/template, None for missing —
|
||||||
|
cheap staleness probe (3 stats) for the load_config fast path."""
|
||||||
|
sig = []
|
||||||
|
for path in (self.config_path, self.secrets_path, self.template_path):
|
||||||
|
try:
|
||||||
|
st = os.stat(path)
|
||||||
|
sig.append((st.st_mtime_ns, st.st_size))
|
||||||
|
except OSError:
|
||||||
|
sig.append(None)
|
||||||
|
return tuple(sig)
|
||||||
|
|
||||||
def load_config(self) -> Dict[str, Any]:
|
def load_config(self) -> Dict[str, Any]:
|
||||||
"""Load configuration from JSON files."""
|
"""Load configuration from JSON files.
|
||||||
|
|
||||||
|
Fast path: when config.json, config_secrets.json and the template
|
||||||
|
are all unchanged since the last successful load (mtime_ns + size),
|
||||||
|
the already-parsed self.config is returned without touching the
|
||||||
|
files — same aliasing semantics as the full path, which also
|
||||||
|
returns self.config.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
|
current_sig = self._files_signature()
|
||||||
|
if self.config and self._loaded_sig == current_sig:
|
||||||
|
return self.config
|
||||||
|
|
||||||
# Check if config file exists, if not create from template
|
# Check if config file exists, if not create from template
|
||||||
if not os.path.exists(self.config_path):
|
if not os.path.exists(self.config_path):
|
||||||
self._create_config_from_template()
|
self._create_config_from_template()
|
||||||
|
|
||||||
# Load main config
|
# Load main config
|
||||||
self.logger.info(f"Attempting to load config from: {os.path.abspath(self.config_path)}")
|
self.logger.info(f"Attempting to load config from: {os.path.abspath(self.config_path)}")
|
||||||
with open(self.config_path, 'r') as f:
|
with open(self.config_path, 'r') as f:
|
||||||
@@ -205,7 +243,10 @@ class ConfigManager:
|
|||||||
self.logger.warning(f"Secrets file not readable ({self.secrets_path}): {e}. Continuing without secrets.")
|
self.logger.warning(f"Secrets file not readable ({self.secrets_path}): {e}. Continuing without secrets.")
|
||||||
except (json.JSONDecodeError, OSError) as e:
|
except (json.JSONDecodeError, OSError) as e:
|
||||||
self.logger.warning(f"Error reading secrets file ({self.secrets_path}): {e}. Continuing without secrets.")
|
self.logger.warning(f"Error reading secrets file ({self.secrets_path}): {e}. Continuing without secrets.")
|
||||||
|
|
||||||
|
# Signature taken AFTER load + migration (migration may write the
|
||||||
|
# config back), so it reflects exactly what was read/written.
|
||||||
|
self._loaded_sig = self._files_signature()
|
||||||
return self.config
|
return self.config
|
||||||
|
|
||||||
except FileNotFoundError as e:
|
except FileNotFoundError as e:
|
||||||
@@ -264,7 +305,8 @@ class ConfigManager:
|
|||||||
json.dump(config_to_write, f, indent=4)
|
json.dump(config_to_write, f, indent=4)
|
||||||
|
|
||||||
# Update the in-memory config to the new state (which includes secrets for runtime)
|
# Update the in-memory config to the new state (which includes secrets for runtime)
|
||||||
self.config = new_config_data
|
self.config = new_config_data
|
||||||
|
self._loaded_sig = self._files_signature()
|
||||||
self.logger.info(f"Configuration successfully saved to {os.path.abspath(self.config_path)}")
|
self.logger.info(f"Configuration successfully saved to {os.path.abspath(self.config_path)}")
|
||||||
if secrets_content:
|
if secrets_content:
|
||||||
self.logger.info("Secret values were preserved in memory and not written to the main config file.")
|
self.logger.info("Secret values were preserved in memory and not written to the main config file.")
|
||||||
|
|||||||
@@ -0,0 +1,129 @@
|
|||||||
|
"""Tests for load_config's mtime fast path (src/config_manager.py).
|
||||||
|
|
||||||
|
load_config used to re-read + re-parse config.json, the template (with a
|
||||||
|
recursive migration diff) and secrets on EVERY call — ~30 web request
|
||||||
|
handlers call it, some 2-3x per request. The fast path skips all of it
|
||||||
|
when the three files' (mtime_ns, size) signatures are unchanged.
|
||||||
|
|
||||||
|
The invariant that matters most: cross-process freshness — a save from
|
||||||
|
the web process must be picked up by the display process's next load.
|
||||||
|
That's guaranteed because the signature is re-stat'd on every call.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import json
|
||||||
|
import os
|
||||||
|
import sys
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||||
|
|
||||||
|
from src.config_manager import ConfigManager # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def mgr(tmp_path):
|
||||||
|
config = tmp_path / "config.json"
|
||||||
|
secrets = tmp_path / "secrets.json"
|
||||||
|
template = tmp_path / "template.json"
|
||||||
|
config.write_text(json.dumps({"display": {"brightness": 90}, "timezone": "UTC"}))
|
||||||
|
secrets.write_text(json.dumps({"weather": {"api_key": "sek"}}))
|
||||||
|
template.write_text(json.dumps({"display": {"brightness": 90}, "timezone": "UTC"}))
|
||||||
|
m = ConfigManager(config_path=str(config), secrets_path=str(secrets))
|
||||||
|
m.template_path = str(template)
|
||||||
|
return m, config, secrets, template
|
||||||
|
|
||||||
|
|
||||||
|
def _count_opens(monkeypatch, mgr_paths):
|
||||||
|
"""Count open() calls hitting the config files."""
|
||||||
|
counts = {"n": 0}
|
||||||
|
real_open = open
|
||||||
|
|
||||||
|
def counting_open(file, *args, **kwargs):
|
||||||
|
if str(file) in mgr_paths:
|
||||||
|
counts["n"] += 1
|
||||||
|
return real_open(file, *args, **kwargs)
|
||||||
|
|
||||||
|
import builtins
|
||||||
|
monkeypatch.setattr(builtins, "open", counting_open)
|
||||||
|
return counts
|
||||||
|
|
||||||
|
|
||||||
|
class TestFastPath:
|
||||||
|
def test_unchanged_files_are_not_reread(self, mgr, monkeypatch):
|
||||||
|
m, config, secrets, template = mgr
|
||||||
|
first = m.load_config()
|
||||||
|
assert first["weather"]["api_key"] == "sek" # secrets merged
|
||||||
|
counts = _count_opens(monkeypatch, {str(config), str(secrets), str(template)})
|
||||||
|
for _ in range(10):
|
||||||
|
again = m.load_config()
|
||||||
|
assert counts["n"] == 0, "fast path must not re-open any config file"
|
||||||
|
assert again is first # same aliasing semantics as the full path
|
||||||
|
|
||||||
|
def test_config_change_triggers_reload(self, mgr):
|
||||||
|
m, config, secrets, template = mgr
|
||||||
|
m.load_config()
|
||||||
|
data = json.loads(config.read_text())
|
||||||
|
data["display"]["brightness"] = 55
|
||||||
|
config.write_text(json.dumps(data))
|
||||||
|
os.utime(config, (os.stat(config).st_atime, os.stat(config).st_mtime + 2))
|
||||||
|
assert m.load_config()["display"]["brightness"] == 55
|
||||||
|
|
||||||
|
def test_secrets_change_triggers_reload(self, mgr):
|
||||||
|
m, config, secrets, template = mgr
|
||||||
|
m.load_config()
|
||||||
|
secrets.write_text(json.dumps({"weather": {"api_key": "NEW"}}))
|
||||||
|
os.utime(secrets, (os.stat(secrets).st_atime, os.stat(secrets).st_mtime + 2))
|
||||||
|
assert m.load_config()["weather"]["api_key"] == "NEW"
|
||||||
|
|
||||||
|
def test_template_change_triggers_reload_and_migration(self, mgr):
|
||||||
|
m, config, secrets, template = mgr
|
||||||
|
m.load_config()
|
||||||
|
template.write_text(json.dumps({
|
||||||
|
"display": {"brightness": 90}, "timezone": "UTC",
|
||||||
|
"brand_new_key": {"added": True}}))
|
||||||
|
os.utime(template, (os.stat(template).st_atime, os.stat(template).st_mtime + 2))
|
||||||
|
reloaded = m.load_config()
|
||||||
|
assert reloaded.get("brand_new_key") == {"added": True}
|
||||||
|
|
||||||
|
def test_same_second_edit_detected_via_mtime_ns_or_size(self, mgr):
|
||||||
|
"""Coarse-mtime same-second edits: size difference still busts it."""
|
||||||
|
m, config, secrets, template = mgr
|
||||||
|
m.load_config()
|
||||||
|
st = os.stat(config)
|
||||||
|
data = json.loads(config.read_text())
|
||||||
|
data["timezone"] = "America/New_York" # different byte length
|
||||||
|
config.write_text(json.dumps(data))
|
||||||
|
os.utime(config, (st.st_atime, st.st_mtime)) # force same mtime
|
||||||
|
assert m.load_config()["timezone"] == "America/New_York"
|
||||||
|
|
||||||
|
|
||||||
|
class TestSaveCoherence:
|
||||||
|
def test_save_config_then_load_returns_saved_data(self, mgr, monkeypatch):
|
||||||
|
m, config, secrets, template = mgr
|
||||||
|
m.load_config()
|
||||||
|
new = {"display": {"brightness": 42}, "timezone": "UTC",
|
||||||
|
"weather": {"api_key": "sek"}}
|
||||||
|
m.save_config(new)
|
||||||
|
counts = _count_opens(monkeypatch, {str(config), str(secrets), str(template)})
|
||||||
|
loaded = m.load_config()
|
||||||
|
assert loaded["display"]["brightness"] == 42
|
||||||
|
assert loaded["weather"]["api_key"] == "sek" # secrets survive in memory
|
||||||
|
assert counts["n"] == 0 # signature refreshed by save; no re-read
|
||||||
|
|
||||||
|
def test_cross_process_save_is_picked_up(self, mgr):
|
||||||
|
"""Another process writing config.json (different mtime) must bust
|
||||||
|
this process's fast path — the core cross-process guarantee."""
|
||||||
|
m, config, secrets, template = mgr
|
||||||
|
m.load_config()
|
||||||
|
other = ConfigManager(config_path=str(config), secrets_path=str(secrets))
|
||||||
|
other.template_path = str(template)
|
||||||
|
other.load_config()
|
||||||
|
other.save_config({"display": {"brightness": 11}, "timezone": "UTC",
|
||||||
|
"weather": {"api_key": "sek"}})
|
||||||
|
os.utime(config, (os.stat(config).st_atime, os.stat(config).st_mtime + 2))
|
||||||
|
assert m.load_config()["display"]["brightness"] == 11
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
sys.exit(pytest.main([__file__, "-v"]))
|
||||||
Reference in New Issue
Block a user