Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions crates/fbuild-daemon/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ pub const SELF_EVICTION_TIMEOUT: Duration = SELF_EVICTION_TIMEOUT_DEFAULT;
/// read from the `FBUILD_WATCH_SET_CACHE_SECS` env var at daemon
/// startup (#122 follow-up).
///
/// - Unset / unparseable → 2 s (the cache's own default).
/// - Unset / unparseable → 0 s (the safe cache default).
/// - Positive integer → that many seconds.
/// - `0` → `Duration::ZERO`, i.e. every entry is stale the instant
/// it's stored. Lets an operator bypass the cache at runtime —
/// useful for A/B-ing a suspected regression without a rebuild.
pub fn watch_set_cache_window_from_env() -> Duration {
const DEFAULT_SECS: u64 = 2;
const DEFAULT_SECS: u64 = 0;
let secs = std::env::var("FBUILD_WATCH_SET_CACHE_SECS")
.ok()
.and_then(|s| s.trim().parse::<u64>().ok())
Expand Down Expand Up @@ -697,7 +697,7 @@ mod tests {

// Unset → default.
unsafe { std::env::remove_var("FBUILD_WATCH_SET_CACHE_SECS") };
assert_eq!(watch_set_cache_window_from_env(), Duration::from_secs(2));
assert_eq!(watch_set_cache_window_from_env(), Duration::ZERO);

// Zero → zero duration (disables the cache by making every
// entry stale on store).
Expand All @@ -710,7 +710,7 @@ mod tests {

// Garbage → default (graceful degradation, not a panic).
unsafe { std::env::set_var("FBUILD_WATCH_SET_CACHE_SECS", "not-a-number") };
assert_eq!(watch_set_cache_window_from_env(), Duration::from_secs(2));
assert_eq!(watch_set_cache_window_from_env(), Duration::ZERO);

// Whitespace around a valid value → parsed after trim.
unsafe { std::env::set_var("FBUILD_WATCH_SET_CACHE_SECS", " 9 ") };
Expand Down
45 changes: 26 additions & 19 deletions crates/fbuild-daemon/src/watch_set_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
//!
//! Implements [`fbuild_build::build_fingerprint::WatchSetStampCache`]
//! over a `DashMap` keyed by a stable hash of the watch set's root
//! paths. Entries are invalidated by a freshness window so a long-
//! running daemon doesn't serve a stale "no changes" answer to a
//! warm-rebuild call that comes minutes after the last one.
//! paths. The production default forces a fresh walk for every build:
//! staging can happen immediately before deployment, and returning a
//! cached "no changes" answer would flash stale firmware.
//!
//! # Why
//!
Expand All @@ -14,14 +14,10 @@
//! that walk is the dominant cost on warm rebuilds — see
//! `docs/PERF_WARM_BUILD.md`.
//!
//! Within the same daemon lifetime a back-to-back `fbuild build` /
//! `fbuild deploy` round-trip can reuse the previous walk's result if
//! it's only a few seconds old: any source change a user just made
//! arrived through the file system, which already advanced the watch
//! root's mtime — but our heuristic deliberately doesn't try to be
//! that precise. A short freshness window (default 2 s, see
//! [`DEFAULT_FRESHNESS`]) is enough for the warm-loop case while
//! keeping the worst-case "ignored a real change" window human-noticeable.
//! Operators can explicitly configure a positive freshness window for
//! controlled performance experiments. It is unsafe for normal deploys:
//! an immediate source edit or AutoResearch staging step can otherwise reuse
//! the preceding fingerprint.
//!
//! # Cycle / staleness model
//!
Expand Down Expand Up @@ -59,12 +55,11 @@ pub struct WatchSetCacheStats {
pub puts: u64,
}

/// Default freshness window for cache entries. Short enough that a
/// user editing a file and immediately re-building still triggers
/// the real walk (modulo edit speed), long enough to cover the
/// back-to-back deploy / re-deploy interaction the sub-1 s budget
/// targets. Override per-instance via [`DaemonWatchSetCache::with_max_age`].
pub const DEFAULT_FRESHNESS: Duration = Duration::from_secs(2);
/// Default freshness window for cache entries. Zero forces a source walk on
/// every deploy so an immediate staging step cannot reuse a stale firmware
/// fingerprint. Override per-instance via [`DaemonWatchSetCache::with_max_age`]
/// only for controlled performance experiments.
pub const DEFAULT_FRESHNESS: Duration = Duration::ZERO;

/// In-memory cache. Cheap to clone via `Arc` because the only
/// state is a `DashMap`. Counter fields are `AtomicU64` so the
Expand Down Expand Up @@ -211,7 +206,7 @@ mod tests {
/// window. Two distinct watch sets must not collide.
#[test]
fn put_then_get_returns_same_hash() {
let cache = DaemonWatchSetCache::new();
let cache = DaemonWatchSetCache::with_max_age(Duration::from_secs(1));
let ws_a = vec![watch("/a")];
let ws_b = vec![watch("/b")];
cache.put(&ws_a, "AAA".to_string());
Expand All @@ -220,11 +215,23 @@ mod tests {
assert_eq!(cache.get(&ws_b).as_deref(), Some("BBB"));
}

#[test]
fn default_cache_does_not_reuse_a_fresh_hash_after_staging() {
let cache = DaemonWatchSetCache::new();
let watches = vec![watch("/project/src/sketch")];
cache.put(&watches, "before-staging".to_string());

assert!(
cache.get(&watches).is_none(),
"the default must force a source walk so an immediate staged change cannot deploy stale firmware"
);
}

/// Same set of paths in different order hashes to the same key —
/// orchestrator can hand us watches without sorting.
#[test]
fn key_is_order_insensitive() {
let cache = DaemonWatchSetCache::new();
let cache = DaemonWatchSetCache::with_max_age(Duration::from_secs(1));
let ws_ab = vec![watch("/a"), watch("/b")];
let ws_ba = vec![watch("/b"), watch("/a")];
cache.put(&ws_ab, "X".to_string());
Expand Down
Loading