perf(agent): symbol index tak pegang mutex global saat rebuild I/O
Sebelumnya SemanticSearch/ListSymbols/RebuildIndex menahan SYMBOL_INDEX Mutex selama full `rebuild` (walk seluruh workspace, bisa detikan) + selama search. Di main loop yang menjalankan read-only tools paralel, semantic_search/list_symbols lain jadi BLOCK selama rebuild. Refactor: - Global berubah Mutex<Option<SymbolIndex>> -> OnceLock<Mutex<HashMap< workspace, SymbolIndex>>> — index per-workspace, jadi pencarian workspace B tidak mungkin bocor simbol stale dari A (workspace-awareness kini struktural, bukan hanya via needs_rebuild). - ensure_symbol_index(workspace, force): rebuild dijalankan DI LUAR lock (mutex hanya dicek/insert/lookup singkat), lalu hasilnya di-swap-in di bawah short lock. Search/list/rebuild-report tak lagi memblock thread lain selama walk I/O. Per-workspace key menghilangkan race lintas-workspace dari skema swap tunggal. - Test +1 (test_ensure_symbol_index_per_workspace_isolation): verifikasi dua workspace punya index independen, rebuild A tidak menimpa B. Verifikasi: check/clippy -D warnings/fmt clean; test infra 64 (0 gagal).
This commit is contained in:
asepharyana
Generated
+11
-11
@@ -4862,7 +4862,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-api"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
@@ -4885,7 +4885,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-application"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"base64",
|
||||
@@ -4903,7 +4903,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-bootstrap"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"chrono",
|
||||
@@ -4920,7 +4920,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-daemon"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"base64",
|
||||
@@ -4944,7 +4944,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-domain"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"base64",
|
||||
@@ -4960,7 +4960,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-gateway"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"axum",
|
||||
@@ -4987,7 +4987,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-grpc"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"axum",
|
||||
@@ -5004,7 +5004,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-infrastructure"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
@@ -5052,7 +5052,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-tui"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"base64",
|
||||
@@ -5078,7 +5078,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-web"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"axum",
|
||||
@@ -5098,7 +5098,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "zesdex-ws"
|
||||
version = "1.20.2"
|
||||
version = "1.21.0"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"axum",
|
||||
|
||||
@@ -121,7 +121,66 @@ pub struct CodeSymbol {
|
||||
}
|
||||
|
||||
/// The in-memory symbol index, shared via a global static.
|
||||
static SYMBOL_INDEX: Mutex<Option<SymbolIndex>> = Mutex::new(None);
|
||||
///
|
||||
/// Keyed by workspace path: each workspace owns its own `SymbolIndex`, so
|
||||
/// searching one repo never leaks stale symbols from another, and a rebuild
|
||||
/// triggered for workspace A cannot clobber the index of B. The lock is only
|
||||
/// ever held briefly (to check/insert/look up) — never across the I/O-heavy
|
||||
/// walk in `rebuild`, which runs locally and is swapped in under a short lock.
|
||||
static SYMBOL_INDEX: std::sync::OnceLock<Mutex<HashMap<String, SymbolIndex>>> =
|
||||
std::sync::OnceLock::new();
|
||||
|
||||
/// Access the (lazily initialised) global per-workspace symbol index map.
|
||||
fn symbol_index_map() -> &'static Mutex<HashMap<String, SymbolIndex>> {
|
||||
SYMBOL_INDEX.get_or_init(|| Mutex::new(HashMap::new()))
|
||||
}
|
||||
|
||||
/// Ensure the per-workspace symbol index is built, returning the symbol count.
|
||||
///
|
||||
/// If `force` is true, or the workspace has no cached (non-empty) index yet,
|
||||
/// the index is rebuilt. The rebuild itself runs OUTSIDE the global lock
|
||||
/// (the walk can take seconds on a large repo), then the result is stored
|
||||
/// under a short lock so concurrent searches never block on the I/O. Returns
|
||||
/// the number of symbols now cached for the workspace.
|
||||
fn ensure_symbol_index(workspace: &str, force: bool) -> Result<usize> {
|
||||
let ready = {
|
||||
let map = symbol_index_map()
|
||||
.lock()
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))?;
|
||||
!force && map.get(workspace).is_some_and(|i| !i.is_empty())
|
||||
};
|
||||
|
||||
if !ready {
|
||||
// Rebuild locally, off the global lock (I/O heavy).
|
||||
let mut fresh = SymbolIndex::new();
|
||||
let count = fresh.rebuild(workspace)?;
|
||||
// Swap in under a short lock; keep an existing non-empty index if a
|
||||
// concurrent rebuild already populated this workspace.
|
||||
let mut map = symbol_index_map()
|
||||
.lock()
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))?;
|
||||
if map.get(workspace).is_none_or(|i| i.is_empty()) {
|
||||
map.insert(workspace.to_string(), fresh);
|
||||
}
|
||||
return Ok(count);
|
||||
}
|
||||
|
||||
let map = symbol_index_map()
|
||||
.lock()
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))?;
|
||||
Ok(map.get(workspace).map_or(0, |i| i.len()))
|
||||
}
|
||||
|
||||
/// Lock and return a borrow to the global per-workspace symbol index map.
|
||||
///
|
||||
/// The caller must have called [`ensure_symbol_index`] first, then looks up
|
||||
/// its workspace key; the lookup is short and in-memory, so holding the guard
|
||||
/// for the search is fine.
|
||||
fn symbol_index() -> Result<std::sync::MutexGuard<'static, HashMap<String, SymbolIndex>>> {
|
||||
symbol_index_map()
|
||||
.lock()
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Language-specific regexes (lazily compiled)
|
||||
@@ -1261,15 +1320,11 @@ impl Tool for SemanticSearch {
|
||||
"semantic search"
|
||||
);
|
||||
|
||||
let mut guard = SYMBOL_INDEX
|
||||
.lock()
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))?;
|
||||
let index = guard.get_or_insert_with(SymbolIndex::new);
|
||||
|
||||
if rebuild || index.needs_rebuild(&workspace) {
|
||||
let count = index.rebuild(&workspace)?;
|
||||
debug!(symbol_count = count, "symbol index rebuilt");
|
||||
}
|
||||
// Build (or load) the per-workspace index without holding the global
|
||||
// lock across the I/O-heavy walk.
|
||||
let _count = ensure_symbol_index(&workspace, rebuild)?;
|
||||
let map = symbol_index()?;
|
||||
let index = map.get(&workspace).expect("index should be ensured");
|
||||
|
||||
// Map kind filter to enum
|
||||
let target_kind = match kind_filter {
|
||||
@@ -1409,12 +1464,15 @@ impl Tool for RebuildIndex {
|
||||
|
||||
info!("rebuilding multi-language symbol index");
|
||||
|
||||
let mut guard = SYMBOL_INDEX
|
||||
.lock()
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))?;
|
||||
let index = guard.get_or_insert_with(SymbolIndex::new);
|
||||
let count = index.rebuild(&workspace)?;
|
||||
let by_lang = index.count_by_language();
|
||||
// Force a rebuild of this workspace's index. The walk runs off the
|
||||
// global lock (via ensure_symbol_index) so it cannot stall concurrent
|
||||
// searches.
|
||||
let count = ensure_symbol_index(&workspace, true)?;
|
||||
let map = symbol_index()?;
|
||||
let by_lang = match map.get(&workspace) {
|
||||
Some(i) => i.count_by_language(),
|
||||
None => Vec::new(),
|
||||
};
|
||||
|
||||
let mut out = format!(
|
||||
"Symbol index rebuilt successfully. {} symbols indexed.\n\n",
|
||||
@@ -1508,15 +1566,11 @@ impl Tool for ListSymbols {
|
||||
.map(|p| p.to_string_lossy().to_string())
|
||||
.unwrap_or_else(|| ".".to_string());
|
||||
|
||||
let mut guard = SYMBOL_INDEX
|
||||
.lock()
|
||||
.map_err(|e| anyhow::anyhow!("index lock failed: {e}"))?;
|
||||
let index = guard.get_or_insert_with(SymbolIndex::new);
|
||||
|
||||
if rebuild || index.needs_rebuild(&workspace) {
|
||||
let count = index.rebuild(&workspace)?;
|
||||
info!(symbol_count = count, "symbol index rebuilt for list");
|
||||
}
|
||||
// Build (or load) the per-workspace index without holding the global
|
||||
// lock across the I/O-heavy walk.
|
||||
let _count = ensure_symbol_index(&workspace, rebuild)?;
|
||||
let map = symbol_index()?;
|
||||
let index = map.get(&workspace).expect("index should be ensured");
|
||||
|
||||
let target_lang = match lang_filter {
|
||||
"rust" => Some(Language::Rust),
|
||||
@@ -1791,4 +1845,46 @@ mod tests {
|
||||
std::fs::remove_dir_all(&ws_a).ok();
|
||||
std::fs::remove_dir_all(&ws_b).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_ensure_symbol_index_per_workspace_isolation() {
|
||||
let ws_a = std::env::temp_dir().join(format!("iso_a_{}", uuid::Uuid::new_v4()));
|
||||
let ws_b = std::env::temp_dir().join(format!("iso_b_{}", uuid::Uuid::new_v4()));
|
||||
std::fs::create_dir_all(&ws_a).unwrap();
|
||||
std::fs::create_dir_all(&ws_b).unwrap();
|
||||
std::fs::write(ws_a.join("a.rs"), "pub fn only_in_a() {}\n").unwrap();
|
||||
std::fs::write(ws_b.join("b.rs"), "pub fn only_in_b() {}\n").unwrap();
|
||||
|
||||
let a = ws_a.to_string_lossy().to_string();
|
||||
let b = ws_b.to_string_lossy().to_string();
|
||||
|
||||
// Build A and B independently through the shared global helper.
|
||||
let count_a = ensure_symbol_index(&a, false).unwrap();
|
||||
assert!(
|
||||
count_a >= 1,
|
||||
"workspace A should index its fn, got {count_a}"
|
||||
);
|
||||
let count_b = ensure_symbol_index(&b, false).unwrap();
|
||||
assert!(
|
||||
count_b >= 1,
|
||||
"workspace B should index its fn, got {count_b}"
|
||||
);
|
||||
|
||||
// Rebuilding A must not have clobbered B and vice-versa.
|
||||
let count_a_again = ensure_symbol_index(&a, true).unwrap();
|
||||
assert!(count_a_again >= 1);
|
||||
|
||||
// Each workspace's cached index is independently correct.
|
||||
{
|
||||
let map = symbol_index().unwrap();
|
||||
let idx_a = map.get(&a).unwrap();
|
||||
assert!(!idx_a.search("only_in_a", 5).is_empty());
|
||||
assert!(idx_a.search("only_in_b", 5).is_empty());
|
||||
let idx_b = map.get(&b).unwrap();
|
||||
assert!(!idx_b.search("only_in_b", 5).is_empty());
|
||||
}
|
||||
|
||||
std::fs::remove_dir_all(&ws_a).ok();
|
||||
std::fs::remove_dir_all(&ws_b).ok();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user