From 22b96c354615384117186f81ddb280b4ee3e8977 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 13 Sep 2026 20:48:52 +0700 Subject: [PATCH] signed_state: Simplify refresh coalescing and stop blocking the executor --- crates/signed_state/src/backend.rs | 11 ++++- crates/signed_state/src/checkouts.rs | 15 +++++-- crates/signed_state/src/refresh.rs | 62 +++++++++++++++++++++------ crates/workspace/src/views/inbox.rs | 1 - docs/over-optimization-action-plan.md | 15 ++++--- 5 files changed, 79 insertions(+), 25 deletions(-) diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 3e6193c..95d30e5 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -5,7 +5,7 @@ use std::time::{Duration, Instant}; use anyhow::{Error, anyhow, bail}; use bitcoin_hashes::sha1::Hash as Sha1Hash; -use gpui::{App, AppContext, Context, Entity, EventEmitter, Global, Task}; +use gpui::{App, AppContext, BackgroundExecutor, Context, Entity, EventEmitter, Global, Task}; use nostr::event::IntoEventBuilder; use nostr_connect::prelude::*; use nostr_sdk::client::SyncSummary; @@ -531,6 +531,7 @@ impl Backend { let repo_id = repo_id.clone(); let servers = servers.clone(); let refs = refs.clone(); + let executor = cx.background_executor().clone(); async move { push_staged_to_grasps( &client, @@ -541,6 +542,7 @@ impl Backend { &destination, &owner, &servers, + &executor, signed_git::push_main, ) .await @@ -686,6 +688,7 @@ impl Backend { let servers = servers.clone(); let refs = refs.clone(); let head = head.clone(); + let executor = cx.background_executor().clone(); async move { push_staged_to_grasps( &client, @@ -696,6 +699,7 @@ impl Backend { &path, &owner, &servers, + &executor, signed_git::push_all, ) .await @@ -854,6 +858,7 @@ impl Backend { let relays = relays.clone(); let refs = refs.clone(); let head = head.clone(); + let executor = cx.background_executor().clone(); async move { push_staged_to_grasps( &client, @@ -864,6 +869,7 @@ impl Backend { &path, &owner, &relays, + &executor, signed_git::push_all, ) .await @@ -1700,6 +1706,7 @@ async fn push_staged_to_grasps( path: &Path, owner: &str, servers: &[RelayUrl], + executor: &BackgroundExecutor, push: fn(&Path, &str, &str, &str) -> Result<(), Error>, ) -> PushOutcome { let mut outcome = PushOutcome::default(); @@ -1722,7 +1729,7 @@ async fn push_staged_to_grasps( 'server: for attempt in 1..=GRASP_PUSH_ATTEMPTS { if attempt > 1 { // Give the server's ingest a moment before re-staging. - std::thread::sleep(GRASP_RETRY_DELAY); + executor.timer(GRASP_RETRY_DELAY).await; } let (event, created_at) = diff --git a/crates/signed_state/src/checkouts.rs b/crates/signed_state/src/checkouts.rs index f7ed9ee..3f6dd50 100644 --- a/crates/signed_state/src/checkouts.rs +++ b/crates/signed_state/src/checkouts.rs @@ -93,6 +93,8 @@ pub struct CheckoutsStore { /// A recompute defaults the base the same way. requested_head: HashMap>, refresh: RefreshGate, + /// True while the timer between a scheduled refresh and its run is pending. + debounce_pending: bool, local_pending: bool, /// When the last full pass (with a remote refresh) completed. /// @@ -163,6 +165,7 @@ impl CheckoutsStore { push_statuses: HashMap::new(), requested_head: HashMap::new(), refresh: RefreshGate::default(), + debounce_pending: false, local_pending: false, last_full_sync: None, _subscriptions: subscriptions, @@ -279,12 +282,15 @@ impl CheckoutsStore { /// Re-resolve the associations and the requested statuses. /// - /// Requests arriving while a pass runs fold into a follow-up. + /// Requests arriving while a pass runs fold into a follow-up, requests + /// arriving while the debounce timer is pending are dropped. pub fn refresh(&mut self, cx: &mut Context) { - if self.refresh.request() != RefreshRequest::Schedule { + if self.debounce_pending || self.refresh.request() != RefreshRequest::Schedule { return; } + self.debounce_pending = true; + cx.spawn(async move |this, cx| { cx.background_executor().timer(REFRESH_DEBOUNCE).await; this.update(cx, |this, cx| this.run_refresh(cx)) @@ -300,6 +306,7 @@ impl CheckoutsStore { /// remote reconciliation cadence ([`Self::local_tick`]); they also restart /// the fast local pass. fn run_refresh(&mut self, cx: &mut Context) { + self.debounce_pending = false; self.refresh.begin(); let records = { @@ -453,7 +460,7 @@ impl CheckoutsStore { } // A full pass or a fresh request covers this tick, skip it. - if self.refresh.running() || self.refresh.debouncing() { + if self.refresh.running() || self.debounce_pending { self.schedule_local_pass(cx); return; } @@ -515,7 +522,7 @@ impl CheckoutsStore { this.update(cx, |this, cx| { // A full pass or a fresh request will apply fresher data // (the tracking refs move only when a full pass fetches). - if this.refresh.running() || this.refresh.debouncing() { + if this.refresh.running() || this.debounce_pending { return; } diff --git a/crates/signed_state/src/refresh.rs b/crates/signed_state/src/refresh.rs index d2564b9..e527e0f 100644 --- a/crates/signed_state/src/refresh.rs +++ b/crates/signed_state/src/refresh.rs @@ -3,14 +3,13 @@ pub struct RefreshGate { running: bool, dirty: bool, - debouncing: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum RefreshRequest { - /// No run or timer covers the request, start the debounce timer. + /// No run covers the request, start one now. Schedule, - /// A run or pending timer already covers the request. + /// A run is in flight and covers the request, fold it into a follow-up. Fold, } @@ -19,28 +18,20 @@ impl RefreshGate { self.running } - pub fn debouncing(&self) -> bool { - self.debouncing - } - /// A new refresh request arrived. /// - /// Folded into a follow-up run while one is in flight, dropped while the - /// debounce timer is pending, otherwise starts the timer. + /// Folded into a follow-up run while one is in flight, otherwise the + /// caller starts the run itself. pub fn request(&mut self) -> RefreshRequest { if self.running { self.dirty = true; RefreshRequest::Fold - } else if self.debouncing { - RefreshRequest::Fold } else { - self.debouncing = true; RefreshRequest::Schedule } } pub fn begin(&mut self) { - self.debouncing = false; self.running = true; } @@ -55,3 +46,48 @@ impl RefreshGate { self.running = false; } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_request_while_running_folds_into_a_follow_up() { + let mut gate = RefreshGate::default(); + gate.begin(); + + assert_eq!(gate.request(), RefreshRequest::Fold); + assert!(gate.finish()); + } + + #[test] + fn a_request_without_a_run_schedules() { + let mut gate = RefreshGate::default(); + + assert_eq!(gate.request(), RefreshRequest::Schedule); + assert!(!gate.running()); + } + + #[test] + fn a_request_after_a_run_schedules_again() { + let mut gate = RefreshGate::default(); + gate.begin(); + assert_eq!(gate.request(), RefreshRequest::Fold); + assert!(gate.finish()); + + assert_eq!(gate.request(), RefreshRequest::Schedule); + } + + #[test] + fn abort_keeps_the_pending_request() { + let mut gate = RefreshGate::default(); + gate.begin(); + assert_eq!(gate.request(), RefreshRequest::Fold); + + gate.abort(); + assert!(!gate.running()); + + gate.begin(); + assert!(gate.finish()); + } +} diff --git a/crates/workspace/src/views/inbox.rs b/crates/workspace/src/views/inbox.rs index fd062ad..ad96522 100644 --- a/crates/workspace/src/views/inbox.rs +++ b/crates/workspace/src/views/inbox.rs @@ -177,7 +177,6 @@ impl InboxView { } fn refresh_initial(&mut self, cx: &mut Context) { - debug_assert!(!self.refresh.debouncing()); if self.refresh.running() { self.refresh.request(); return; diff --git a/docs/over-optimization-action-plan.md b/docs/over-optimization-action-plan.md index 5d93d30..ba0f447 100644 --- a/docs/over-optimization-action-plan.md +++ b/docs/over-optimization-action-plan.md @@ -1,6 +1,6 @@ # Over-optimization and over-engineering action plan -- **Status:** phase 1 implemented; phases 2 and 3 still draft, for manual review +- **Status:** phases 1 and 2 implemented; phase 3 still draft, for manual review - **Date:** 2026-09-13 - **Basis:** optimization inventory audit of branch `remove-ai-slop` (HEAD `534d572`) - **Scope:** non-UI crates only — `signed_state`, `signed_git`, `signed_core`, `utils`, @@ -123,6 +123,8 @@ Manual checks for phases 2 and 3 are in the validation section at the end. scanning coalesces; inbox refresh after a sync does not double-run. - **Size:** M. **Risk:** medium. The debounce timing of `CheckoutsStore` must not regress; the timer is preserved exactly, only its flag moves. +- **Status:** done 2026-09-13 — `RefreshGate` is `running`/`dirty` only, `CheckoutsStore` owns + `debounce_pending`, and the one-line `inbox.rs` adaptation landed here. ### OV-04 — Replace the blocking sleep in the grasp push retry @@ -140,6 +142,8 @@ Manual checks for phases 2 and 3 are in the validation section at the end. - **Acceptance:** `cargo check -p signed_state`; a push that hits a transient denial still retries with the same spacing. - **Size:** S. **Risk:** low. +- **Status:** done 2026-09-13 — `push_staged_to_grasps` takes a `BackgroundExecutor` and awaits + `executor.timer(GRASP_RETRY_DELAY)`. --- @@ -288,7 +292,8 @@ Renumbering, for traceability from the first review round: ## Open questions for the owner -1. OV-03: accept the two-flag `RefreshGate` plus a `CheckoutsStore`-owned debounce flag, or - keep the shared state machine as documentation-only? -2. OV-08: is an approximate (`20000+`) commit total acceptable for the badge contract? -3. OV-03: who owns the one mechanical `inbox.rs` edit — this plan or the UI effort? +1. OV-08: is an approximate (`20000+`) commit total acceptable for the badge contract? + +OV-03's questions were settled during implementation: the gate keeps two flags (`running`, +`dirty`), `CheckoutsStore` owns `debounce_pending`, and the mechanical `inbox.rs` edit landed +in this phase.