From 27179b1e937d587fbece2eb4f565baff4f790229 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 28 Apr 2026 17:19:57 +1000 Subject: [PATCH 1/7] fix(perf): parallelize provider inventory entries loop Replace the sequential `for` loop in `entries()` with `futures::future::join_all` so all 47 providers resolve their identity and configuration concurrently. Each `entry_for_provider` call is read-only (registry read lock, config reads, SQLite SELECT), so parallel execution is safe. This should reduce per-call time from ~1.6s to ~200ms (bounded by the slowest single provider at ~208ms). Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/goose/src/providers/inventory/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/goose/src/providers/inventory/mod.rs b/crates/goose/src/providers/inventory/mod.rs index 7db08a06b26d..d739b71fcf83 100644 --- a/crates/goose/src/providers/inventory/mod.rs +++ b/crates/goose/src/providers/inventory/mod.rs @@ -11,6 +11,7 @@ use sqlx::{Pool, Row, Sqlite, Transaction}; use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::Arc; use tokio::sync::RwLock; +use futures::future::join_all; const STALE_AFTER_HOURS: i64 = 24; @@ -231,12 +232,11 @@ impl ProviderInventoryService { pub async fn entries(&self, provider_ids: &[String]) -> Result> { let ids = self.resolve_provider_ids(provider_ids).await; - let mut entries = Vec::with_capacity(ids.len()); - for provider_id in ids { - if let Some(entry) = self.entry_for_provider(&provider_id).await? { - entries.push(entry); - } - } + let results = join_all(ids.iter().map(|id| self.entry_for_provider(id))).await; + let entries: Vec<_> = results + .into_iter() + .filter_map(|r| r.ok().flatten()) + .collect(); Ok(entries) } From c1e101397882ffd2a44a47f6c94a532267f2557d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 29 Apr 2026 15:34:21 +1000 Subject: [PATCH 2/7] fix(perf): eagerly initialize SQLite pool to overlap with provider resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SQLite pool uses connect_lazy_with, so initialization (schema check, migrations) only triggers on first use — currently inside read_snapshot during provider inventory. This adds ~1.6s to the critical path. Spawn a background task to call pool() immediately after SessionManager creation, so the pool initializes concurrently with provider resolution (~811ms) and describe_provider work. By the time read_snapshot or listSessions actually needs the pool, the OnceCell should already be resolved. This is safe because pool() uses get_or_try_init which handles concurrent callers correctly. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/goose/src/acp/server.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index d8c9fb35dcfc..2d5f6f129c6f 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -848,6 +848,15 @@ impl GooseAcpAgent { goose_platform: GoosePlatform, ) -> Result { let session_manager = Arc::new(SessionManager::new(data_dir)); + + // Eagerly initialize the SQLite pool so it's ready when providers/sessions need it. + // The pool uses connect_lazy_with, so initialization only happens on first use. + // By spawning this now, the ~1.6s schema check overlaps with provider resolution. + let storage_clone = session_manager.storage().clone(); + tokio::spawn(async move { + let _ = storage_clone.pool().await; + }); + let thread_manager = Arc::new(crate::session::ThreadManager::new( session_manager.storage().clone(), )); From 2ed8c7d459044fe31d63cfb5b7680307ad4ec42b Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 29 Apr 2026 15:54:38 +1000 Subject: [PATCH 3/7] fix(perf): use tokio::spawn for true parallel provider resolution join_all runs all futures on the same tokio task, only interleaving at .await points. Since describe_provider does blocking work (config reads, hash computation) between awaits, the futures executed sequentially. Use tokio::spawn to dispatch each provider onto a separate thread from the tokio thread pool, enabling actual concurrent execution across all 47 providers. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/goose/src/providers/inventory/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/goose/src/providers/inventory/mod.rs b/crates/goose/src/providers/inventory/mod.rs index d739b71fcf83..304a75788daa 100644 --- a/crates/goose/src/providers/inventory/mod.rs +++ b/crates/goose/src/providers/inventory/mod.rs @@ -11,7 +11,6 @@ use sqlx::{Pool, Row, Sqlite, Transaction}; use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::Arc; use tokio::sync::RwLock; -use futures::future::join_all; const STALE_AFTER_HOURS: i64 = 24; @@ -232,10 +231,13 @@ impl ProviderInventoryService { pub async fn entries(&self, provider_ids: &[String]) -> Result> { let ids = self.resolve_provider_ids(provider_ids).await; - let results = join_all(ids.iter().map(|id| self.entry_for_provider(id))).await; - let entries: Vec<_> = results + let handles: Vec<_> = ids.into_iter().map(|id| { + let this = self.clone(); + tokio::spawn(async move { this.entry_for_provider(&id).await }) + }).collect(); + let entries: Vec<_> = futures::future::join_all(handles).await .into_iter() - .filter_map(|r| r.ok().flatten()) + .filter_map(|r| r.ok().and_then(|r| r.ok()).flatten()) .collect(); Ok(entries) } From 542eb5b486bc5c8dd852492e1b6859a1ef7fc26c Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 29 Apr 2026 16:44:18 +1000 Subject: [PATCH 4/7] refactor: simplify eager pool init comment to single line Remove implementation detail lines from the comment about eager SQLite pool initialization, keeping only the intent description per code review feedback. Signed-off-by: Matt Toohey --- crates/goose/src/acp/server.rs | 2 -- crates/goose/src/providers/inventory/mod.rs | 14 +++++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 2d5f6f129c6f..3471e93cdbf3 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -850,8 +850,6 @@ impl GooseAcpAgent { let session_manager = Arc::new(SessionManager::new(data_dir)); // Eagerly initialize the SQLite pool so it's ready when providers/sessions need it. - // The pool uses connect_lazy_with, so initialization only happens on first use. - // By spawning this now, the ~1.6s schema check overlaps with provider resolution. let storage_clone = session_manager.storage().clone(); tokio::spawn(async move { let _ = storage_clone.pool().await; diff --git a/crates/goose/src/providers/inventory/mod.rs b/crates/goose/src/providers/inventory/mod.rs index 304a75788daa..6ebaab922f9a 100644 --- a/crates/goose/src/providers/inventory/mod.rs +++ b/crates/goose/src/providers/inventory/mod.rs @@ -231,11 +231,15 @@ impl ProviderInventoryService { pub async fn entries(&self, provider_ids: &[String]) -> Result> { let ids = self.resolve_provider_ids(provider_ids).await; - let handles: Vec<_> = ids.into_iter().map(|id| { - let this = self.clone(); - tokio::spawn(async move { this.entry_for_provider(&id).await }) - }).collect(); - let entries: Vec<_> = futures::future::join_all(handles).await + let handles: Vec<_> = ids + .into_iter() + .map(|id| { + let this = self.clone(); + tokio::spawn(async move { this.entry_for_provider(&id).await }) + }) + .collect(); + let entries: Vec<_> = futures::future::join_all(handles) + .await .into_iter() .filter_map(|r| r.ok().and_then(|r| r.ok()).flatten()) .collect(); From be152ca3599ea6b69626b21453cc106292ce028a Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 29 Apr 2026 20:17:56 +1000 Subject: [PATCH 5/7] fix(perf): restore error propagation in parallel entries() The filter_map chain silently discarded JoinErrors (task panics) and anyhow::Errors (SQLite/config failures), only filtering Ok(None). Replace with explicit error handling that propagates JoinError and inner errors with ?, matching the original sequential behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/goose/src/providers/inventory/mod.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/goose/src/providers/inventory/mod.rs b/crates/goose/src/providers/inventory/mod.rs index 6ebaab922f9a..ebdccf6b4105 100644 --- a/crates/goose/src/providers/inventory/mod.rs +++ b/crates/goose/src/providers/inventory/mod.rs @@ -3,7 +3,7 @@ use super::canonical::{map_provider_name, map_to_canonical_model, CanonicalModel use crate::config::declarative_providers::{DeclarativeProviderConfig, ProviderEngine}; use crate::config::Config; use crate::session::session_manager::SessionStorage; -use anyhow::Result; +use anyhow::{Context, Result}; use chrono::{DateTime, Duration, Utc}; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; @@ -238,11 +238,14 @@ impl ProviderInventoryService { tokio::spawn(async move { this.entry_for_provider(&id).await }) }) .collect(); - let entries: Vec<_> = futures::future::join_all(handles) - .await - .into_iter() - .filter_map(|r| r.ok().and_then(|r| r.ok()).flatten()) - .collect(); + let results = futures::future::join_all(handles).await; + let mut entries = Vec::with_capacity(results.len()); + for result in results { + let inner = result.context("provider inventory task panicked")?; + if let Some(entry) = inner? { + entries.push(entry); + } + } Ok(entries) } From e24e758cf584a40a1a2d63dff7b824817ab497d9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 29 Apr 2026 21:27:37 +1000 Subject: [PATCH 6/7] fix(perf): tie eager pool init to agent lifecycle Store the pool warmup JoinHandle and abort it in Drop, preventing the fire-and-forget task from outliving the agent (causing nondeterministic cleanup failures in tests). Also log a warning if eager init fails, instead of silently discarding the error. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/goose/src/acp/server.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 3471e93cdbf3..ae5357361c55 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -193,6 +193,13 @@ pub struct GooseAcpAgent { disable_session_naming: bool, provider_inventory: ProviderInventoryService, goose_platform: GoosePlatform, + pool_warmup: tokio::task::JoinHandle<()>, +} + +impl Drop for GooseAcpAgent { + fn drop(&mut self) { + self.pool_warmup.abort(); + } } /// Shorten a session/thread id for perf log correlation. @@ -851,8 +858,10 @@ impl GooseAcpAgent { // Eagerly initialize the SQLite pool so it's ready when providers/sessions need it. let storage_clone = session_manager.storage().clone(); - tokio::spawn(async move { - let _ = storage_clone.pool().await; + let pool_warmup = tokio::spawn(async move { + if let Err(e) = storage_clone.pool().await { + tracing::warn!("Eager pool init failed (will retry on first use): {e}"); + } }); let thread_manager = Arc::new(crate::session::ThreadManager::new( @@ -876,6 +885,7 @@ impl GooseAcpAgent { disable_session_naming, provider_inventory, goose_platform, + pool_warmup, }) } From b481cabdb29df4bf6adbdc9f6c3ef156d6449027 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 29 Apr 2026 21:43:11 +1000 Subject: [PATCH 7/7] Revert "fix(perf): tie eager pool init to agent lifecycle" This reverts commit e24e758cf584a40a1a2d63dff7b824817ab497d9. --- crates/goose/src/acp/server.rs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index ae5357361c55..3471e93cdbf3 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -193,13 +193,6 @@ pub struct GooseAcpAgent { disable_session_naming: bool, provider_inventory: ProviderInventoryService, goose_platform: GoosePlatform, - pool_warmup: tokio::task::JoinHandle<()>, -} - -impl Drop for GooseAcpAgent { - fn drop(&mut self) { - self.pool_warmup.abort(); - } } /// Shorten a session/thread id for perf log correlation. @@ -858,10 +851,8 @@ impl GooseAcpAgent { // Eagerly initialize the SQLite pool so it's ready when providers/sessions need it. let storage_clone = session_manager.storage().clone(); - let pool_warmup = tokio::spawn(async move { - if let Err(e) = storage_clone.pool().await { - tracing::warn!("Eager pool init failed (will retry on first use): {e}"); - } + tokio::spawn(async move { + let _ = storage_clone.pool().await; }); let thread_manager = Arc::new(crate::session::ThreadManager::new( @@ -885,7 +876,6 @@ impl GooseAcpAgent { disable_session_naming, provider_inventory, goose_platform, - pool_warmup, }) }