diff --git a/.bifrost/suppressions.json b/.bifrost/suppressions.json index e96a3266..9948abc5 100644 --- a/.bifrost/suppressions.json +++ b/.bifrost/suppressions.json @@ -133,18 +133,6 @@ "accepted_at": "2026-08-04", "expires_at": null }, - { - "policy_id": "bifrost.performance.serialization-in-loop", - "finding_id": "2175df8014fc064443d2425243532bed8e2ad9ee917ef165c8cd7e9a31adbfcf", - "path": "crates/bifrost-analysis/src/searchtools/scan_usages.rs", - "identity_stability": "strong", - "status": "accepted", - "reason": "Each iteration serializes a distinct item; the serialization is inherent to the loop and cannot be hoisted.", - "policy_hash_at_acceptance": "da0b97c9fccd69df43320804e18b2325952a603f136ad01af388816b2a49a4b4", - "accepted_by": "dbakereffendi", - "accepted_at": "2026-08-04", - "expires_at": null - }, { "policy_id": "bifrost.performance.sleep-in-loop", "finding_id": "fe639e787c5ab14fa523237e759da98dab58a64b440d731bbf5eccdc97e91ae2", diff --git a/crates/bifrost-analysis/src/analyzer/analyzer_definition_lookup.rs b/crates/bifrost-analysis/src/analyzer/analyzer_definition_lookup.rs index a3d80e37..1d408cd4 100644 --- a/crates/bifrost-analysis/src/analyzer/analyzer_definition_lookup.rs +++ b/crates/bifrost-analysis/src/analyzer/analyzer_definition_lookup.rs @@ -444,8 +444,8 @@ impl<'a> AnalyzerDefinitionLookup<'a> { matches } - /// Resolve many rendered names into the shared fqn memo with two batched - /// relational reads per language instead of one point batch per name. + /// Resolve rendered names in one language into the shared fqn memo with + /// two batched relational reads instead of one point batch per name. /// /// The rounds are the same two questions [`Self::exact_for_language`] /// asks per name -- an exact persisted-identity seek, then the identifier @@ -454,95 +454,100 @@ impl<'a> AnalyzerDefinitionLookup<'a> { /// what the point path would compute. A cancelled or failed batch /// memoizes nothing: every name stays unmemoized and the point path /// retries it with unchanged results. - pub(crate) fn prefetch_fqns(&self, fqns: &[String]) { - for language in self.query_languages() { - let missing: Vec = { - let cache = self - .memo - .fqn_cache - .lock() - .expect("definition fqn cache poisoned"); - let mut seen = HashSet::default(); - fqns.iter() - .filter(|fqn| seen.insert(fqn.as_str())) - .filter(|fqn| !cache.contains_key(&(language, (*fqn).clone()))) - .cloned() - .collect() - }; - if missing.is_empty() { - continue; - } + pub(crate) fn prefetch_fqn_in_language(&self, language: Language, fqns: &[String]) { + let missing: Vec = { + let cache = self + .memo + .fqn_cache + .lock() + .expect("definition fqn cache poisoned"); + let mut seen = HashSet::default(); + fqns.iter() + .filter(|fqn| seen.insert(fqn.as_str())) + .filter(|fqn| !cache.contains_key(&(language, (*fqn).clone()))) + .cloned() + .collect() + }; + if missing.is_empty() { + return; + } - let mut exact_owners = Vec::new(); - let mut exact_questions = Vec::new(); - for (index, fqn) in missing.iter().enumerate() { - if let Some(name) = Self::rendered_name(language, fqn) { - exact_owners.push(index); - exact_questions.push((name, RelationalDefinitionQuery::ExactName)); - } + let mut exact_owners = Vec::new(); + let mut exact_questions = Vec::new(); + for (index, fqn) in missing.iter().enumerate() { + if let Some(name) = Self::rendered_name(language, fqn) { + exact_owners.push(index); + exact_questions.push((name, RelationalDefinitionQuery::ExactName)); } - // A name the language cannot even render as a path resolves to - // nothing without a fallback, exactly as the point path answers. - let mut parseable = vec![false; missing.len()]; - let mut units_by_name: Vec> = vec![Vec::new(); missing.len()]; - if !exact_questions.is_empty() { - let expected = exact_questions.len(); - let values = self.query_values(language, exact_questions); - if values.len() != expected { - return; - } - for (owner, value) in exact_owners.into_iter().zip(values) { - parseable[owner] = true; - match value { - RelationalDefinitionValue::Definitions(units) => { - units_by_name[owner] = units; - } - _ => panic!("an exact-name query returned the wrong result shape"), + } + // A name the language cannot even render as a path resolves to + // nothing without a fallback, exactly as the point path answers. + let mut parseable = vec![false; missing.len()]; + let mut units_by_name: Vec> = vec![Vec::new(); missing.len()]; + if !exact_questions.is_empty() { + let expected = exact_questions.len(); + let values = self.query_values(language, exact_questions); + if values.len() != expected { + return; + } + for (owner, value) in exact_owners.into_iter().zip(values) { + parseable[owner] = true; + match value { + RelationalDefinitionValue::Definitions(units) => { + units_by_name[owner] = units; } + _ => panic!("an exact-name query returned the wrong result shape"), } } + } - let mut fallback: Vec<(usize, std::ops::Range, Vec)> = Vec::new(); - let mut fallback_questions = Vec::new(); - for (index, fqn) in missing.iter().enumerate() { - units_by_name[index].retain(|unit| unit.fq_name() == *fqn); - if !parseable[index] || !units_by_name[index].is_empty() { - continue; - } - let identifiers = self.rendered_identifier_candidates(language, fqn); - let start = fallback_questions.len(); - fallback_questions.extend(Self::identifier_queries(language, &identifiers, None)); - fallback.push((index, start..fallback_questions.len(), identifiers)); + let mut fallback: Vec<(usize, std::ops::Range, Vec)> = Vec::new(); + let mut fallback_questions = Vec::new(); + for (index, fqn) in missing.iter().enumerate() { + units_by_name[index].retain(|unit| unit.fq_name() == *fqn); + if !parseable[index] || !units_by_name[index].is_empty() { + continue; } - if !fallback_questions.is_empty() { - let expected = fallback_questions.len(); - let values = self.query_values(language, fallback_questions); - if values.len() != expected { - return; - } - let mut values = values.into_iter().map(Some).collect::>(); - for (index, range, identifiers) in fallback { - let name_values = values[range] - .iter_mut() - .map(|value| value.take().expect("each fallback value is consumed once")) - .collect::>(); - units_by_name[index] = - Self::identifier_units_from_values(&identifiers, name_values); - units_by_name[index].retain(|unit| unit.fq_name() == missing[index]); - } + let identifiers = self.rendered_identifier_candidates(language, fqn); + let start = fallback_questions.len(); + fallback_questions.extend(Self::identifier_queries(language, &identifiers, None)); + fallback.push((index, start..fallback_questions.len(), identifiers)); + } + if !fallback_questions.is_empty() { + let expected = fallback_questions.len(); + let values = self.query_values(language, fallback_questions); + if values.len() != expected { + return; } - - let mut cache = self - .memo - .fqn_cache - .lock() - .expect("definition fqn cache poisoned"); - for (fqn, mut units) in missing.into_iter().zip(units_by_name) { - sort_units(&mut units); - units.dedup(); - cache.insert((language, fqn), units); + let mut values = values.into_iter().map(Some).collect::>(); + for (index, range, identifiers) in fallback { + let name_values = values[range] + .iter_mut() + .map(|value| value.take().expect("each fallback value is consumed once")) + .collect::>(); + units_by_name[index] = + Self::identifier_units_from_values(&identifiers, name_values); + units_by_name[index].retain(|unit| unit.fq_name() == missing[index]); } } + + let mut cache = self + .memo + .fqn_cache + .lock() + .expect("definition fqn cache poisoned"); + for (fqn, mut units) in missing.into_iter().zip(units_by_name) { + sort_units(&mut units); + units.dedup(); + cache.insert((language, fqn), units); + } + } + + /// Resolve many rendered names across every supported language. + pub(crate) fn prefetch_fqns(&self, fqns: &[String]) { + for language in self.query_languages() { + self.prefetch_fqn_in_language(language, fqns); + } } } diff --git a/crates/bifrost-analysis/src/analyzer/cpp/imports.rs b/crates/bifrost-analysis/src/analyzer/cpp/imports.rs index e0c86df0..8f472595 100644 --- a/crates/bifrost-analysis/src/analyzer/cpp/imports.rs +++ b/crates/bifrost-analysis/src/analyzer/cpp/imports.rs @@ -96,6 +96,13 @@ pub enum HeaderLanguageAttribution { impl TestDetectionProvider for CppAnalyzer {} impl ImportAnalysisProvider for CppAnalyzer { + fn import_infos_for_files( + &self, + files: &[ProjectFile], + ) -> Option>> { + Some(self.inner.bulk_import_infos(files.iter().cloned())) + } + fn file_dependency_facts_for_files( &self, files: &[ProjectFile], diff --git a/crates/bifrost-analysis/src/analyzer/cpp/mod.rs b/crates/bifrost-analysis/src/analyzer/cpp/mod.rs index 4207ea38..acaa4a37 100644 --- a/crates/bifrost-analysis/src/analyzer/cpp/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/cpp/mod.rs @@ -1632,6 +1632,10 @@ impl IAnalyzer for CppAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -1824,6 +1828,42 @@ impl IAnalyzer for CppAnalyzer { #[cfg(any(test, feature = "test-support"))] impl crate::analyzer::AnalyzerTestHooks for CppAnalyzer { + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/csharp/mod.rs b/crates/bifrost-analysis/src/analyzer/csharp/mod.rs index a83928d4..be053841 100644 --- a/crates/bifrost-analysis/src/analyzer/csharp/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/csharp/mod.rs @@ -1108,6 +1108,10 @@ impl IAnalyzer for CSharpAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -1318,6 +1322,18 @@ impl crate::analyzer::AnalyzerTestHooks for CSharpAnalyzer { .definition_candidates_query_count_for_test() } + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/go/imports.rs b/crates/bifrost-analysis/src/analyzer/go/imports.rs index 090e7391..45a2fc2d 100644 --- a/crates/bifrost-analysis/src/analyzer/go/imports.rs +++ b/crates/bifrost-analysis/src/analyzer/go/imports.rs @@ -23,6 +23,13 @@ use super::GoAnalyzer; use crate::analyzer::{AnalyzerQueryScope, QueryScope}; impl ImportAnalysisProvider for GoAnalyzer { + fn import_infos_for_files( + &self, + files: &[ProjectFile], + ) -> Option>> { + Some(self.inner.bulk_import_infos(files.iter().cloned())) + } + fn file_dependency_facts_for_files( &self, files: &[ProjectFile], diff --git a/crates/bifrost-analysis/src/analyzer/go/mod.rs b/crates/bifrost-analysis/src/analyzer/go/mod.rs index 4c7085cf..41f55ab4 100644 --- a/crates/bifrost-analysis/src/analyzer/go/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/go/mod.rs @@ -793,6 +793,10 @@ impl IAnalyzer for GoAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -1015,6 +1019,42 @@ impl crate::analyzer::AnalyzerTestHooks for GoAnalyzer { .evaluation_root_continuation_semantic_cache_revivals_for_test() } + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/i_analyzer.rs b/crates/bifrost-analysis/src/analyzer/i_analyzer.rs index e8f4fa16..ed8e1296 100644 --- a/crates/bifrost-analysis/src/analyzer/i_analyzer.rs +++ b/crates/bifrost-analysis/src/analyzer/i_analyzer.rs @@ -1037,6 +1037,16 @@ pub trait IAnalyzer: CodeUnitIndex + Send + Sync + Any { false } + /// Best-effort batch-warm the request-scoped `definitions()` memo for + /// many names at once. A caller that already knows a name superset (for + /// instance every declaration a whole-workspace scan already enumerated) + /// can call this once so later individual `definitions()` lookups against + /// those same names -- inside `get_definition`'s per-occurrence + /// resolution, for instance -- hit a warm memo instead of paying one + /// relational round trip each. A no-op with no open query boundary, and + /// for analyzers without a `definitions()` memo to warm. + fn prefetch_definitions(&self, _fq_names: &[String]) {} + /// The cancellation token carried by the innermost active query boundary. /// /// Compatibility APIs such as `CodeUnitIndex::definitions` cannot accept a @@ -1888,6 +1898,18 @@ pub trait AnalyzerTestHooks { 0 } + /// Relational-store round trips issued by `RelationalDefinitionLookup::batch`, + /// one per call regardless of how many requests it carried. Paired with + /// a test that also counts the distinct names it resolved, to show "one + /// batched call for many names" instead of "one call per name" (bifrost#15). + #[doc(hidden)] + fn reset_relational_definition_batch_call_count_for_test(&self) {} + + #[doc(hidden)] + fn relational_definition_batch_call_count_for_test(&self) -> usize { + 0 + } + /// Store round trips the definition-candidate row read actually issued, /// as distinct from the calls that were served by the request's /// single-flight memo. diff --git a/crates/bifrost-analysis/src/analyzer/java/mod.rs b/crates/bifrost-analysis/src/analyzer/java/mod.rs index daeae779..7124eb50 100644 --- a/crates/bifrost-analysis/src/analyzer/java/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/java/mod.rs @@ -718,6 +718,10 @@ impl IAnalyzer for JavaAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -1019,6 +1023,42 @@ impl crate::analyzer::AnalyzerTestHooks for JavaAnalyzer { .java_usage_evidence_cache_stats_for_test() } + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/javascript/mod.rs b/crates/bifrost-analysis/src/analyzer/javascript/mod.rs index c4fffe1d..dd28d640 100644 --- a/crates/bifrost-analysis/src/analyzer/javascript/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/javascript/mod.rs @@ -595,6 +595,10 @@ impl IAnalyzer for JavascriptAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -794,6 +798,42 @@ impl IAnalyzer for JavascriptAnalyzer { #[cfg(any(test, feature = "test-support"))] impl crate::analyzer::AnalyzerTestHooks for JavascriptAnalyzer { + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/kotlin/imports.rs b/crates/bifrost-analysis/src/analyzer/kotlin/imports.rs index c230b49b..df56cfba 100644 --- a/crates/bifrost-analysis/src/analyzer/kotlin/imports.rs +++ b/crates/bifrost-analysis/src/analyzer/kotlin/imports.rs @@ -154,6 +154,13 @@ impl KotlinAnalyzer { } impl ImportAnalysisProvider for KotlinAnalyzer { + fn import_infos_for_files( + &self, + files: &[ProjectFile], + ) -> Option>> { + Some(self.inner.bulk_import_infos(files.iter().cloned())) + } + fn file_dependency_facts_for_files( &self, files: &[ProjectFile], diff --git a/crates/bifrost-analysis/src/analyzer/kotlin/mod.rs b/crates/bifrost-analysis/src/analyzer/kotlin/mod.rs index ce4606e6..99f66f47 100644 --- a/crates/bifrost-analysis/src/analyzer/kotlin/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/kotlin/mod.rs @@ -792,6 +792,10 @@ impl IAnalyzer for KotlinAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -998,6 +1002,42 @@ impl IAnalyzer for KotlinAnalyzer { #[cfg(any(test, feature = "test-support"))] impl crate::analyzer::AnalyzerTestHooks for KotlinAnalyzer { + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/multi_analyzer.rs b/crates/bifrost-analysis/src/analyzer/multi_analyzer.rs index 3fa63880..0e292212 100644 --- a/crates/bifrost-analysis/src/analyzer/multi_analyzer.rs +++ b/crates/bifrost-analysis/src/analyzer/multi_analyzer.rs @@ -1614,6 +1614,12 @@ impl IAnalyzer for MultiAnalyzer { self.attached_read_ledgers.load(Ordering::Relaxed) > 0 } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.delegates + .values() + .for_each(|delegate| delegate.analyzer().prefetch_definitions(fq_names)); + } + fn active_query_cancellation(&self) -> Option { self.query_contexts .lock() @@ -2515,6 +2521,48 @@ impl crate::analyzer::AnalyzerTestHooks for MultiAnalyzer { .sum() } + fn reset_definition_prefetch_batch_count_for_test(&self) { + for delegate in self.delegates.values() { + delegate + .analyzer() + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.delegates + .values() + .map(|delegate| { + delegate + .analyzer() + .test_hooks() + .definition_prefetch_batch_count_for_test() + }) + .sum() + } + + fn reset_relational_definition_batch_call_count_for_test(&self) { + for delegate in self.delegates.values() { + delegate + .analyzer() + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.delegates + .values() + .map(|delegate| { + delegate + .analyzer() + .test_hooks() + .relational_definition_batch_call_count_for_test() + }) + .sum() + } + fn reset_full_declaration_scan_count_for_test(&self) { for delegate in self.delegates.values() { delegate diff --git a/crates/bifrost-analysis/src/analyzer/php/mod.rs b/crates/bifrost-analysis/src/analyzer/php/mod.rs index ba531de1..ecba2b8a 100644 --- a/crates/bifrost-analysis/src/analyzer/php/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/php/mod.rs @@ -513,6 +513,10 @@ impl IAnalyzer for PhpAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -690,6 +694,42 @@ impl IAnalyzer for PhpAnalyzer { #[cfg(any(test, feature = "test-support"))] impl crate::analyzer::AnalyzerTestHooks for PhpAnalyzer { + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/python/mod.rs b/crates/bifrost-analysis/src/analyzer/python/mod.rs index 02deaeb6..5044738d 100644 --- a/crates/bifrost-analysis/src/analyzer/python/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/python/mod.rs @@ -691,6 +691,10 @@ impl IAnalyzer for PythonAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -896,6 +900,42 @@ impl IAnalyzer for PythonAnalyzer { #[cfg(any(test, feature = "test-support"))] impl crate::analyzer::AnalyzerTestHooks for PythonAnalyzer { + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/ruby/imports.rs b/crates/bifrost-analysis/src/analyzer/ruby/imports.rs index 8bc23d46..b589fdca 100644 --- a/crates/bifrost-analysis/src/analyzer/ruby/imports.rs +++ b/crates/bifrost-analysis/src/analyzer/ruby/imports.rs @@ -46,6 +46,13 @@ impl RubyAnalyzer { } impl ImportAnalysisProvider for RubyAnalyzer { + fn import_infos_for_files( + &self, + files: &[ProjectFile], + ) -> Option>> { + Some(self.inner.bulk_import_infos(files.iter().cloned())) + } + fn file_dependency_facts_for_files( &self, files: &[ProjectFile], diff --git a/crates/bifrost-analysis/src/analyzer/ruby/mod.rs b/crates/bifrost-analysis/src/analyzer/ruby/mod.rs index 03221154..2c834d60 100644 --- a/crates/bifrost-analysis/src/analyzer/ruby/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/ruby/mod.rs @@ -489,6 +489,10 @@ impl IAnalyzer for RubyAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -665,6 +669,42 @@ impl IAnalyzer for RubyAnalyzer { #[cfg(any(test, feature = "test-support"))] impl crate::analyzer::AnalyzerTestHooks for RubyAnalyzer { + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/rust/imports.rs b/crates/bifrost-analysis/src/analyzer/rust/imports.rs index 598f0b33..e24b85a4 100644 --- a/crates/bifrost-analysis/src/analyzer/rust/imports.rs +++ b/crates/bifrost-analysis/src/analyzer/rust/imports.rs @@ -18,6 +18,13 @@ use super::RustAnalyzer; use crate::analyzer::{AnalyzerQueryScope, QueryScope}; impl ImportAnalysisProvider for RustAnalyzer { + fn import_infos_for_files( + &self, + files: &[ProjectFile], + ) -> Option>> { + Some(self.inner.bulk_import_infos(files.iter().cloned())) + } + fn file_dependency_facts_for_files( &self, files: &[ProjectFile], diff --git a/crates/bifrost-analysis/src/analyzer/rust/mod.rs b/crates/bifrost-analysis/src/analyzer/rust/mod.rs index f21a3c47..743b5bfd 100644 --- a/crates/bifrost-analysis/src/analyzer/rust/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/rust/mod.rs @@ -1114,6 +1114,10 @@ impl IAnalyzer for RustAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -1383,6 +1387,18 @@ impl crate::analyzer::AnalyzerTestHooks for RustAnalyzer { .definition_candidates_query_count_for_test() } + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/scala/mod.rs b/crates/bifrost-analysis/src/analyzer/scala/mod.rs index d862e2a7..9822fc76 100644 --- a/crates/bifrost-analysis/src/analyzer/scala/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/scala/mod.rs @@ -1631,6 +1631,10 @@ impl IAnalyzer for ScalaAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -1841,6 +1845,42 @@ impl IAnalyzer for ScalaAnalyzer { #[cfg(any(test, feature = "test-support"))] impl crate::analyzer::AnalyzerTestHooks for ScalaAnalyzer { + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/structural/reference_edges.rs b/crates/bifrost-analysis/src/analyzer/structural/reference_edges.rs index 60dc752d..69c5e55d 100644 --- a/crates/bifrost-analysis/src/analyzer/structural/reference_edges.rs +++ b/crates/bifrost-analysis/src/analyzer/structural/reference_edges.rs @@ -460,13 +460,45 @@ impl<'a> ReferenceEngine<'a> { max_usages: usize, max_source_bytes: Option, ) -> ReferenceRun { - let query = self.query_with_provider_and_source_budget( + self.references_to_edges_with_provider( analyzer, targets, None, max_files, max_usages, max_source_bytes, + ) + } + + /// [`Self::references_to_edges`], but lets the caller pick the candidate + /// file provider. + /// + /// `find_default_candidates_within` (the `None` case) chooses the + /// interruptible, per-candidate importer scan specifically so a caller + /// with a real deadline can bail out mid-scan instead of being forced + /// through one uninterruptible workspace-wide reverse-import-index + /// build. A caller whose cancellation token can never actually fire + /// (e.g. a batch scan with no deadline) pays that scan's full cost on + /// every call for a protection it will never use. Passing + /// `Some(&ImportGraphCandidateProvider::new())` opts into the same + /// import-graph candidates through the cached reverse-import-index path + /// instead (bifrost#15). + pub fn references_to_edges_with_provider( + &self, + analyzer: &dyn IAnalyzer, + targets: &[CodeUnit], + explicit_provider: Option<&dyn crate::analyzer::usages::CandidateFileProvider>, + max_files: usize, + max_usages: usize, + max_source_bytes: Option, + ) -> ReferenceRun { + let query = self.query_with_provider_and_source_budget( + analyzer, + targets, + explicit_provider, + max_files, + max_usages, + max_source_bytes, ); let generation = analyzer.project().analysis_generation(); let mut reasons = match query.completion { diff --git a/crates/bifrost-analysis/src/analyzer/tree_sitter_analyzer.rs b/crates/bifrost-analysis/src/analyzer/tree_sitter_analyzer.rs index 1fadf35c..ce0d57f0 100644 --- a/crates/bifrost-analysis/src/analyzer/tree_sitter_analyzer.rs +++ b/crates/bifrost-analysis/src/analyzer/tree_sitter_analyzer.rs @@ -2890,6 +2890,7 @@ pub struct TreeSitterAnalyzer { sql_definitions_query_count: Arc, definition_candidates_query_count: Arc, definition_prefetch_batch_count: Arc, + relational_definition_batch_call_count: Arc, definition_candidate_row_read_count: Arc, /// Candidate spellings dropped by `definition_candidate_short_names` /// because the persisted `short_name` vocabulary for this adapter's @@ -2953,6 +2954,9 @@ impl Clone for TreeSitterAnalyzer { sql_definitions_query_count: Arc::clone(&self.sql_definitions_query_count), definition_candidates_query_count: Arc::clone(&self.definition_candidates_query_count), definition_prefetch_batch_count: Arc::clone(&self.definition_prefetch_batch_count), + relational_definition_batch_call_count: Arc::clone( + &self.relational_definition_batch_call_count, + ), definition_candidate_row_read_count: Arc::clone( &self.definition_candidate_row_read_count, ), @@ -3199,6 +3203,7 @@ where sql_definitions_query_count: Arc::new(AtomicUsize::new(0)), definition_candidates_query_count: Arc::new(AtomicUsize::new(0)), definition_prefetch_batch_count: Arc::new(AtomicUsize::new(0)), + relational_definition_batch_call_count: Arc::new(AtomicUsize::new(0)), definition_candidate_row_read_count: Arc::new(AtomicUsize::new(0)), structural_miss_spelling_count: Arc::new(AtomicUsize::new(0)), enclosing_code_unit_query_count: Arc::new(AtomicUsize::new(0)), @@ -3609,6 +3614,7 @@ where sql_definitions_query_count: Arc::new(AtomicUsize::new(0)), definition_candidates_query_count: Arc::new(AtomicUsize::new(0)), definition_prefetch_batch_count: Arc::new(AtomicUsize::new(0)), + relational_definition_batch_call_count: Arc::new(AtomicUsize::new(0)), definition_candidate_row_read_count: Arc::new(AtomicUsize::new(0)), structural_miss_spelling_count: Arc::new(AtomicUsize::new(0)), enclosing_code_unit_query_count: Arc::new(AtomicUsize::new(0)), @@ -8639,6 +8645,22 @@ where self.definition_prefetch_batch_count.load(Ordering::Relaxed) } + /// Relational-store round trips issued by `RelationalDefinitionLookup::batch`, + /// one per call regardless of how many requests it carried. A caller that + /// resolves many distinct names one at a time drives this as high as the + /// name count; a caller that batches them first keeps it flat (bifrost#15). + #[doc(hidden)] + pub fn reset_relational_definition_batch_call_count_for_test(&self) { + self.relational_definition_batch_call_count + .store(0, Ordering::Relaxed); + } + + #[doc(hidden)] + pub fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.relational_definition_batch_call_count + .load(Ordering::Relaxed) + } + /// Persisted candidate-row reads that actually reached the store, one per /// (short name, ordering) the request has not already read. Paired with /// `definition_candidates_query_count_for_test` it separates "one read for @@ -12249,6 +12271,8 @@ where let values = if unique.is_empty() { Vec::new() } else { + self.relational_definition_batch_call_count + .fetch_add(1, Ordering::Relaxed); let current = self.store_context.store.relational_definition_values( self.adapter.as_ref(), self.project.root(), @@ -12984,6 +13008,10 @@ where TreeSitterAnalyzer::active_query_cancellation(self) } + fn prefetch_definitions(&self, fq_names: &[String]) { + TreeSitterAnalyzer::prefetch_definitions(self, fq_names); + } + fn active_query_semantic_model_overlay( &self, ) -> Option>> { @@ -13459,6 +13487,14 @@ where TreeSitterAnalyzer::definition_prefetch_batch_count_for_test(self) } + fn reset_relational_definition_batch_call_count_for_test(&self) { + TreeSitterAnalyzer::reset_relational_definition_batch_call_count_for_test(self); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + TreeSitterAnalyzer::relational_definition_batch_call_count_for_test(self) + } + fn reset_definition_candidate_row_read_count_for_test(&self) { TreeSitterAnalyzer::reset_definition_candidate_row_read_count_for_test(self); } diff --git a/crates/bifrost-analysis/src/analyzer/typescript/mod.rs b/crates/bifrost-analysis/src/analyzer/typescript/mod.rs index bc105170..d25d4fef 100644 --- a/crates/bifrost-analysis/src/analyzer/typescript/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/typescript/mod.rs @@ -704,6 +704,10 @@ impl IAnalyzer for TypescriptAnalyzer { self.inner.end_query(context); } + fn prefetch_definitions(&self, fq_names: &[String]) { + self.inner.prefetch_definitions(fq_names); + } + fn record_query_failure(&self, error: crate::analyzer::store::StoreError) { self.inner.record_query_failure(error); } @@ -907,6 +911,42 @@ impl IAnalyzer for TypescriptAnalyzer { #[cfg(any(test, feature = "test-support"))] impl crate::analyzer::AnalyzerTestHooks for TypescriptAnalyzer { + fn reset_relational_definition_batch_call_count_for_test(&self) { + self.inner + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + } + + fn relational_definition_batch_call_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .relational_definition_batch_call_count_for_test() + } + + fn reset_definition_candidates_query_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + } + + fn definition_candidates_query_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_candidates_query_count_for_test() + } + + fn reset_definition_prefetch_batch_count_for_test(&self) { + self.inner + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + } + + fn definition_prefetch_batch_count_for_test(&self) -> usize { + self.inner + .test_hooks() + .definition_prefetch_batch_count_for_test() + } + fn reset_full_declaration_scan_count_for_test(&self) { self.inner .test_hooks() diff --git a/crates/bifrost-analysis/src/analyzer/usages/candidates.rs b/crates/bifrost-analysis/src/analyzer/usages/candidates.rs index 0dcef6f8..c6248e11 100644 --- a/crates/bifrost-analysis/src/analyzer/usages/candidates.rs +++ b/crates/bifrost-analysis/src/analyzer/usages/candidates.rs @@ -277,6 +277,24 @@ fn cpp_related_callable_source_files( related } +/// Test-only count of [`find_direct_importers_with_cancellation`] calls, so a +/// caller that switched to the cached-reverse-import-index path can assert it +/// no longer takes the uncached, per-candidate scan (bifrost#15). +#[cfg(test)] +pub(crate) static FIND_DIRECT_IMPORTERS_WITH_CANCELLATION_CALL_COUNT: + std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +#[cfg(test)] +pub(crate) fn reset_find_direct_importers_with_cancellation_call_count_for_test() { + FIND_DIRECT_IMPORTERS_WITH_CANCELLATION_CALL_COUNT + .store(0, std::sync::atomic::Ordering::Relaxed); +} + +#[cfg(test)] +pub(crate) fn find_direct_importers_with_cancellation_call_count_for_test() -> usize { + FIND_DIRECT_IMPORTERS_WITH_CANCELLATION_CALL_COUNT.load(std::sync::atomic::Ordering::Relaxed) +} + fn find_direct_importers_with_cancellation( files: impl IntoIterator, import_provider: &dyn ImportAnalysisProvider, @@ -284,6 +302,9 @@ fn find_direct_importers_with_cancellation( source_files: &BTreeSet, cancellation: &CancellationToken, ) -> HashSet { + #[cfg(test)] + FIND_DIRECT_IMPORTERS_WITH_CANCELLATION_CALL_COUNT + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); let mut files: Vec<_> = files.into_iter().collect(); // Everything from here to the per-candidate loop is workspace-scale and // uninterruptible once entered: sorting every analyzed file, and the diff --git a/crates/bifrost-analysis/src/analyzer/usages/get_definition/mod.rs b/crates/bifrost-analysis/src/analyzer/usages/get_definition/mod.rs index 53de9627..5324a72b 100644 --- a/crates/bifrost-analysis/src/analyzer/usages/get_definition/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/usages/get_definition/mod.rs @@ -998,12 +998,32 @@ fn resolve_definition_requests_traced<'a>( .collect() } +/// Test-only count of [`resolve_definition_batch_with_source`] invocations, +/// so a batching caller can assert it collapsed many per-edge calls into one +/// call per file rather than re-deriving that from timing (bifrost#15). +#[cfg(test)] +pub(crate) static RESOLVE_DEFINITION_BATCH_WITH_SOURCE_CALL_COUNT: std::sync::atomic::AtomicUsize = + std::sync::atomic::AtomicUsize::new(0); + +#[cfg(test)] +pub(crate) fn reset_resolve_definition_batch_with_source_call_count_for_test() { + RESOLVE_DEFINITION_BATCH_WITH_SOURCE_CALL_COUNT.store(0, std::sync::atomic::Ordering::Relaxed); +} + +#[cfg(test)] +pub(crate) fn resolve_definition_batch_with_source_call_count_for_test() -> usize { + RESOLVE_DEFINITION_BATCH_WITH_SOURCE_CALL_COUNT.load(std::sync::atomic::Ordering::Relaxed) +} + pub fn resolve_definition_batch_with_source( analyzer: &dyn IAnalyzer, requests: Vec, file: ProjectFile, source: Arc, ) -> Vec { + #[cfg(test)] + RESOLVE_DEFINITION_BATCH_WITH_SOURCE_CALL_COUNT + .fetch_add(1, std::sync::atomic::Ordering::Relaxed); let scope = AnalyzerQueryScope::new(analyzer); let token = scope.token(); let scope = AnalyzerQueryScope::new(analyzer); diff --git a/crates/bifrost-analysis/src/analyzer/usages/mod.rs b/crates/bifrost-analysis/src/analyzer/usages/mod.rs index e3f4ed6a..f2e233af 100644 --- a/crates/bifrost-analysis/src/analyzer/usages/mod.rs +++ b/crates/bifrost-analysis/src/analyzer/usages/mod.rs @@ -17,7 +17,7 @@ pub mod call_binding; pub mod call_relations; pub mod call_shape; pub mod callable_signature; -mod candidates; +pub(crate) mod candidates; pub(crate) mod common; pub mod cpp_graph; pub mod csharp_graph; diff --git a/crates/bifrost-analysis/src/analyzer/usages/workspace_graph.rs b/crates/bifrost-analysis/src/analyzer/usages/workspace_graph.rs index 48e8d62b..0867dd94 100644 --- a/crates/bifrost-analysis/src/analyzer/usages/workspace_graph.rs +++ b/crates/bifrost-analysis/src/analyzer/usages/workspace_graph.rs @@ -8,6 +8,7 @@ use crate::analyzer::languages::{ use crate::analyzer::{CodeUnit, DeclarationId, IAnalyzer, Language, ProjectFile, Range}; use crate::cancellation::CancellationToken; use crate::hash::{HashMap, HashSet}; +use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; use std::collections::{BTreeMap, BTreeSet}; use std::ffi::OsStr; @@ -136,41 +137,88 @@ impl WorkspaceUsageCatalog { .expect("uncancelled workspace usage catalog construction") } - pub(crate) fn build_with_cancellation( + /// Enumerate one file's graph declarations through its persisted summary + /// projection, the same per-file cache-backed lookup the rooted path + /// (`build_for_files`) already relies on. This is one query per file + /// rather than one query per declaration, and each file's lookup is + /// independent of every other file's, so callers can run it under + /// `rayon::par_iter()` across files (see bifrost#15). + fn declarations_for_file( analyzer: &dyn IAnalyzer, - cancellation: &CancellationToken, - ) -> Option { + file: &ProjectFile, + ) -> Vec<(CodeUnit, Option)> { let mut declarations = Vec::new(); - for (unit, range) in analyzer.all_declarations_with_primary_ranges() { - if cancellation.is_cancelled() { - return None; + if let Some(projection) = analyzer.summary_file_projection(file) { + let mut stack = projection.top_level_declarations.clone(); + let mut seen = HashSet::default(); + while let Some(unit) = stack.pop() { + if !seen.insert(unit.clone()) { + continue; + } + if let Some(children) = projection.children.get(&unit) { + stack.extend(children.iter().cloned()); + } + if is_graph_declaration(&unit) { + declarations.push(( + unit.clone(), + projection + .ranges + .get(&unit) + .and_then(|ranges| primary_range(ranges)), + )); + } } - if is_graph_declaration(&unit) { - declarations.push((unit, range)); + } else { + for unit in analyzer.declarations(file) { + if is_graph_declaration(&unit) { + let range = analyzer.ranges(&unit).into_iter().min_by_key(range_key); + declarations.push((unit, range)); + } } } - // The public declaration inventory intentionally excludes synthetic // file scopes. Java module descriptors need one graph caller, however, // so add the existing `module-info.java` file scope through this // graph-only catalog path. This avoids turning the named module into a // package Module CodeUnit, which can collide with a package of the same // name. - for file in analyzer.analyzed_files() { - if cancellation.is_cancelled() { - return None; - } - if !is_java_module_descriptor_file(&file) { - continue; - } + if is_java_module_descriptor_file(file) { let file_scope = CodeUnit::file_scope(file.clone()); let range = analyzer .ranges(&file_scope) .into_iter() - .min_by_key(|range| (range.start_line, range.start_byte)); + .min_by_key(range_key); declarations.push((file_scope, range)); } + declarations + } + + pub(crate) fn build_with_cancellation( + analyzer: &dyn IAnalyzer, + cancellation: &CancellationToken, + ) -> Option { + if cancellation.is_cancelled() { + return None; + } + let files = analyzer.analyzed_files(); + let declarations: Vec<(CodeUnit, Option)> = { + let _scope = crate::profiling::scope("workspace_graph::parallel_enumeration"); + files + .par_iter() + .filter_map(|file| { + if cancellation.is_cancelled() { + return None; + } + Some(Self::declarations_for_file(analyzer, file)) + }) + .flatten_iter() + .collect() + }; + if cancellation.is_cancelled() { + return None; + } + let _scope = crate::profiling::scope("workspace_graph::from_declarations"); Self::from_declarations(declarations, cancellation) } @@ -179,45 +227,10 @@ impl WorkspaceUsageCatalog { /// enumerate every declaration in a long-lived workspace cache before it can /// answer a handful of changed-file roots. pub(crate) fn build_for_files(analyzer: &dyn IAnalyzer, files: &[ProjectFile]) -> Self { - let mut declarations = Vec::new(); - for file in files { - if let Some(projection) = analyzer.summary_file_projection(file) { - let mut stack = projection.top_level_declarations.clone(); - let mut seen = HashSet::default(); - while let Some(unit) = stack.pop() { - if !seen.insert(unit.clone()) { - continue; - } - if let Some(children) = projection.children.get(&unit) { - stack.extend(children.iter().cloned()); - } - if is_graph_declaration(&unit) { - declarations.push(( - unit.clone(), - projection - .ranges - .get(&unit) - .and_then(|ranges| primary_range(ranges)), - )); - } - } - } else { - for unit in analyzer.declarations(file) { - if is_graph_declaration(&unit) { - let range = analyzer.ranges(&unit).into_iter().min_by_key(range_key); - declarations.push((unit, range)); - } - } - } - if is_java_module_descriptor_file(file) { - let file_scope = CodeUnit::file_scope(file.clone()); - let range = analyzer - .ranges(&file_scope) - .into_iter() - .min_by_key(range_key); - declarations.push((file_scope, range)); - } - } + let declarations = files + .iter() + .flat_map(|file| Self::declarations_for_file(analyzer, file)) + .collect(); Self::from_declarations(declarations, &CancellationToken::default()) .expect("uncancelled rooted workspace usage catalog construction") } diff --git a/crates/bifrost-analysis/src/searchtools/scan_usages.rs b/crates/bifrost-analysis/src/searchtools/scan_usages.rs index 5c0ef300..7e8fba4b 100644 --- a/crates/bifrost-analysis/src/searchtools/scan_usages.rs +++ b/crates/bifrost-analysis/src/searchtools/scan_usages.rs @@ -2823,10 +2823,13 @@ pub fn usage_graph(analyzer: &dyn IAnalyzer, params: UsageGraphParams) -> UsageG } else { eligible_files.clone() }; - let root_catalog = if rooted { - WorkspaceUsageCatalog::build_for_files(analyzer, &root_files) - } else { - WorkspaceUsageCatalog::build(analyzer) + let root_catalog = { + let _scope = profiling::scope("usage_graph::root_catalog_build"); + if rooted { + WorkspaceUsageCatalog::build_for_files(analyzer, &root_files) + } else { + WorkspaceUsageCatalog::build(analyzer) + } }; let mut declarations: Vec<(CodeUnit, Option)> = root_catalog @@ -2860,6 +2863,13 @@ pub fn usage_graph(analyzer: &dyn IAnalyzer, params: UsageGraphParams) -> UsageG let definitions = AnalyzerDefinitionLookup::new(analyzer, Language::None); let mut endpoints_by_name: HashMap<(UsageEcosystem, String), Vec> = HashMap::default(); + // `declarations` only grows within the depth loop (newly-discovered + // targets are pushed onto it as the frontier expands), so the catalog + // built from it is stable within one iteration and only needs rebuilding + // once a later iteration has appended more entries -- not on every + // iteration of `params.depth`, which just re-clones and re-sorts the + // same, unchanged prefix each time. + let mut layer_catalog_once: Option<(usize, WorkspaceUsageCatalog)> = None; for _ in 0..params.depth { if frontier.is_empty() { @@ -2881,21 +2891,23 @@ pub fn usage_graph(analyzer: &dyn IAnalyzer, params: UsageGraphParams) -> UsageG let mut structural_exact_by_site = UsageGraphExactSites::default(); let mut inverse_exact_by_site = UsageGraphExactSites::default(); let mut authoritative_exact_sites: HashSet = HashSet::default(); - let mut structural_exact_loaded = false; let mut inverse_exact_targets: HashSet<(UsageEcosystem, String)> = HashSet::default(); - let layer_declaration_units = declarations - .iter() - .map(|(unit, _)| unit.clone()) - .collect::>(); - let layer_catalog = WorkspaceUsageCatalog::from_declarations( - layer_declaration_units - .iter() - .cloned() - .map(|unit| (unit, None)) - .collect(), - &CancellationToken::default(), - ) - .expect("uncancelled exact layer catalog construction"); + if layer_catalog_once + .as_ref() + .is_none_or(|(built_len, _)| *built_len != declarations.len()) + { + let _scope = profiling::scope("usage_graph::layer_catalog_build"); + let catalog = WorkspaceUsageCatalog::from_declarations( + declarations + .iter() + .map(|(unit, _)| (unit.clone(), None)) + .collect(), + &CancellationToken::default(), + ) + .expect("uncancelled exact layer catalog construction"); + layer_catalog_once = Some((declarations.len(), catalog)); + } + let layer_catalog = &layer_catalog_once.as_ref().unwrap().1; let mut legacy_edges: BTreeMap< (UsageEcosystem, String, String), @@ -2965,15 +2977,31 @@ pub fn usage_graph(analyzer: &dyn IAnalyzer, params: UsageGraphParams) -> UsageG } } Some(crate::analyzer::languages::LanguageEdgeSites::Scoped(result)) => { + // This branch has the same two costs the `Fqn` branch above was + // just fixed for: a store round trip per edge target, and a + // linear scan of `declarations` per edge for the caller. Batch + // and index once per pass instead of once per edge (bifrost#15). + let target_fqns = result + .edges + .keys() + .map(|(_, to)| to.fqn.clone()) + .collect::>(); + for language in [Language::TypeScript, Language::JavaScript] { + definitions.prefetch_fqn_in_language(language, &target_fqns); + } + let mut callers_by_key: HashMap<(ProjectFile, String), Vec> = + HashMap::default(); + for (unit, _) in &declarations { + callers_by_key + .entry((unit.source().clone(), unit.fq_name())) + .or_default() + .push(unit.clone()); + } for ((from, to), sites) in result.edges { - let callers = declarations - .iter() - .map(|(unit, _)| unit) - .filter(|unit| { - unit.source() == &from.file && unit.fq_name() == from.fqn - }) + let callers = callers_by_key + .get(&(from.file.clone(), from.fqn.clone())) .cloned() - .collect::>(); + .unwrap_or_default(); let targets = [Language::TypeScript, Language::JavaScript] .into_iter() .flat_map(|language| definitions.fqn_in_language(&to.fqn, language)) @@ -3036,36 +3064,74 @@ pub fn usage_graph(analyzer: &dyn IAnalyzer, params: UsageGraphParams) -> UsageG .map(|(ecosystem, _, target)| (*ecosystem, target.clone())) .chain(legacy_truncated.keys().cloned()) .collect::>(); - for endpoint_key in endpoint_keys { - endpoints_by_name - .entry(endpoint_key) - .or_insert_with_key(|(ecosystem, to_name)| { - let mut endpoints = declarations - .iter() - .map(|(unit, _)| unit) - .filter(|unit| { - UsageEcosystem::of(language_for_target(unit)) == *ecosystem - && unit.fq_name() == *to_name - }) - .cloned() - .collect::>(); - if endpoints.is_empty() { - endpoints.extend( - ecosystem_languages(*ecosystem) - .iter() - .flat_map(|language| { - definitions.fqn_in_language(to_name, *language) - }) - .filter(is_graph_declaration) - .filter(|unit| { - test_files - .as_ref() - .is_none_or(|exclusion| !exclusion.excludes(unit.source())) - }), - ); - } - endpoints - }); + + // Names with no local-layer match fall back to the relational store, + // one exact-name round trip per distinct name. On a large workspace + // that is thousands of sequential round trips; batch them into one + // call per language instead. See bifrost issue #15. + // + // Grouping every declaration by (ecosystem, fq_name) once turns both + // the "does this exist locally" check below and the endpoint lookup + // further down into O(1) map lookups instead of an O(declarations) + // scan repeated per endpoint key -- O(endpoint_keys * declarations) + // on a large workspace otherwise. See bifrost issue #15. + let mut declarations_by_key: HashMap<(UsageEcosystem, String), Vec> = + HashMap::default(); + { + let _scope = profiling::scope("usage_graph::declarations_by_key_build"); + for (unit, _) in &declarations { + declarations_by_key + .entry(( + UsageEcosystem::of(language_for_target(unit)), + unit.fq_name(), + )) + .or_default() + .push(unit.clone()); + } + } + let mut store_fallback_names: HashMap> = HashMap::default(); + for (ecosystem, to_name) in &endpoint_keys { + if !declarations_by_key.contains_key(&(*ecosystem, to_name.clone())) { + for language in ecosystem_languages(*ecosystem) { + store_fallback_names + .entry(*language) + .or_default() + .push(to_name.clone()); + } + } + } + for (language, names) in store_fallback_names { + definitions.prefetch_fqn_in_language(language, &names); + } + + { + let _scope = profiling::scope("usage_graph::endpoint_keys_resolution"); + for endpoint_key in endpoint_keys { + endpoints_by_name + .entry(endpoint_key) + .or_insert_with_key(|(ecosystem, to_name)| { + let mut endpoints = declarations_by_key + .get(&(*ecosystem, to_name.clone())) + .cloned() + .unwrap_or_default(); + if endpoints.is_empty() { + endpoints.extend( + ecosystem_languages(*ecosystem) + .iter() + .flat_map(|language| { + definitions.fqn_in_language(to_name, *language) + }) + .filter(is_graph_declaration) + .filter(|unit| { + test_files.as_ref().is_none_or(|exclusion| { + !exclusion.excludes(unit.source()) + }) + }), + ); + } + endpoints + }); + } } // The structural exact table below is only ever probed at the site @@ -3090,142 +3156,223 @@ pub fn usage_graph(analyzer: &dyn IAnalyzer, params: UsageGraphParams) -> UsageG }; let mut next = BTreeSet::new(); - for ((ecosystem, from_name, to_name), sites) in legacy_edges { - let endpoint_key = (ecosystem, to_name.clone()); - let mut endpoints = endpoints_by_name[&endpoint_key].clone(); - if unique_graph_unit(&endpoints).is_none() - && inverse_exact_targets.insert(endpoint_key.clone()) + + // A file referenced by many ambiguous edges was hitting the slow + // `get_definition` fallback once per edge, and each call built a + // fresh `DefinitionBatchContext` with empty tree/source caches -- + // re-parsing that file's AST once per edge instead of once total. On + // the k8s reproduction, files touched by the fallback averaged + // 9-17x as many calls as distinct files (and climbing). Deferring + // the slow path and batching it per file below collapses that back + // to one call per file; the fast-path scan (this pass) and the + // per-site finalization (the last pass, below) are unchanged. See + // bifrost#15. + struct DeferredSlowPathSite { + site_key: UsageGraphSiteKey, + from_name: String, + to_name: String, + line: usize, + spans: Vec<(usize, usize)>, + endpoints_snapshot: Vec, + } + let mut pending_slow_path: HashMap> = + HashMap::default(); + + // Every ambiguous target's fast-path scan and exact-reference scan is + // independent of every other target's: site keys embed `to_name`, so + // two different targets can never write the same key, and `endpoints` + // starts fresh from `endpoints_by_name` for each one. On a workspace + // the size of a large monorepo this loop ran hundreds of these scans + // sequentially, each taking anywhere from single-digit milliseconds + // to several seconds -- collectively most of usage_graph's remaining + // runtime after the fixes above. Select the ambiguous targets first + // (cheap, sequential, and must preserve today's "first edge in + // BTreeMap order wins" semantics for a to_name shared by several + // edges), run each target's scan in parallel on the shared heavy-scan + // pool, then merge every result back in sequentially. See bifrost#15. + struct AmbiguousTarget<'a> { + ecosystem: UsageEcosystem, + from_name: &'a str, + to_name: &'a str, + sites: &'a [crate::analyzer::usages::inverted_edges::CallSite], + } + let mut ambiguous_targets: Vec> = Vec::new(); + for ((ecosystem, from_name, to_name), sites) in &legacy_edges { + let endpoint_key = (*ecosystem, to_name.clone()); + if unique_graph_unit(&endpoints_by_name[&endpoint_key]).is_none() + && inverse_exact_targets.insert(endpoint_key) { - for site in &sites { - let site_key = ( - site.path.clone(), - site.line, - from_name.clone(), - to_name.clone(), - ); - if authoritative_exact_sites.contains(&site_key) || site.spans.is_empty() { - continue; - } - let Some(file) = scan_files_by_path.get(&site.path) else { - continue; - }; - if !site.exact_targets.is_empty() { - let (start, end) = site.spans[0]; - let range = Range { - start_byte: start, - end_byte: end, - start_line: site.line, - end_line: site.line, - }; - if let Some(source_unit) = analyzer - .enclosing_code_unit(file, &range) - .filter(|unit| unit.fq_name() == from_name) - && let Some(source_index) = - layer_catalog.index_for_id(&source_unit.declaration_id()) - { - let source_id = layer_catalog.nodes[source_index].key.id.clone(); - let pairs = site - .exact_targets - .iter() - .filter(|target| { - target.fq_name() == to_name && is_graph_declaration(target) - }) - .map(|target| { - if !endpoints.iter().any(|endpoint| { - endpoint.declaration_id() == target.declaration_id() - }) { - endpoints.push(target.clone()); - } - target.clone() - }) - .map(|target| (source_id.clone(), target.declaration_id())) - .collect::>(); - if !pairs.is_empty() { - authoritative_exact_sites.insert(site_key.clone()); - exact_by_site.insert(site_key, pairs); - continue; - } - } - } - let Some(source) = analyzer.indexed_source(file) else { - continue; - }; - let requests = site - .spans - .iter() - .map(|(start, end)| { - crate::analyzer::usages::get_definition::DefinitionLookupRequest { - file: file.clone(), - line: None, - column: None, - start_byte: Some(*start), - end_byte: Some(*end), - } - }) - .collect::>(); - let outcomes = crate::analyzer::usages::get_definition::resolve_definition_batch_with_source( - analyzer, - requests, - file.clone(), - source.into(), - ); - let mut point_pairs = BTreeSet::new(); - for ((start, end), outcome) in site.spans.iter().zip(outcomes) { - if outcome.status - != crate::analyzer::usages::get_definition::DefinitionLookupStatus::Resolved + ambiguous_targets.push(AmbiguousTarget { + ecosystem: *ecosystem, + from_name, + to_name, + sites, + }); + } + } + + if !ambiguous_targets.is_empty() { + let structural = + ReferenceEngine::new().scan_file_edges_at_lines(analyzer, &structural_exact_sites); + if !structural.completeness.is_complete() { + incomplete.insert(( + "exact_reference_join_incomplete".to_string(), + "structural exact reference attribution was incomplete".to_string(), + )); + } + for row in structural.edges { + let Some(source) = row.site.enclosing.as_ref() else { + continue; + }; + let site_key = ( + rel_path_string(&row.site.file), + row.site.range.start_line, + source.fq_name(), + row.target.fq_name(), + ); + if authoritative_exact_sites.contains(&site_key) { + continue; + } + structural_exact_by_site + .entry(site_key) + .or_default() + .insert((source.declaration_id(), row.target_id())); + } + } + + struct AmbiguousTargetResult { + ecosystem: UsageEcosystem, + to_name: String, + endpoints: Vec, + new_authoritative_sites: Vec, + new_exact_by_site: Vec<(UsageGraphSiteKey, BTreeSet)>, + new_pending_slow_path: Vec<(ProjectFile, DeferredSlowPathSite)>, + new_inverse_exact_by_site: UsageGraphExactSites, + incomplete_reasons: Vec<(String, String)>, + } + // The bounded semantic pass deliberately aggregates by its legacy + // graph key. When that key names multiple exact declarations, ask + // the same language plugin's target side to retain the overload + // identity for only this layer's admitted files. This is a rare + // ambiguity join, not a second unconditional workspace scan. + let admitted_files = scan_files.iter().cloned().collect::>(); + // `authoritative_exact_sites` as it stands before this parallel + // section is safe to share read-only: nothing below writes into it + // until the sequential merge, and a key any worker below could ever + // query embeds that worker's own `to_name`, so no worker can + // possibly need to see another worker's (still pending) insertion. + let authoritative_exact_sites_snapshot = &authoritative_exact_sites; + let results: Vec = HEAVY_SCAN_POOL.install(|| { + ambiguous_targets + .par_iter() + .map(|target| { + let endpoint_key = (target.ecosystem, target.to_name.to_string()); + let mut endpoints = endpoints_by_name[&endpoint_key].clone(); + let mut new_authoritative_sites = Vec::new(); + let mut new_exact_by_site = Vec::new(); + let mut new_pending_slow_path = Vec::new(); + for site in target.sites { + let site_key = ( + site.path.clone(), + site.line, + target.from_name.to_string(), + target.to_name.to_string(), + ); + if authoritative_exact_sites_snapshot.contains(&site_key) + || site.spans.is_empty() { continue; } - let range = Range { - start_byte: *start, - end_byte: *end, - start_line: site.line, - end_line: site.line, - }; - let Some(source_unit) = analyzer - .enclosing_code_unit(file, &range) - .filter(|unit| unit.fq_name() == from_name) - else { - continue; - }; - let Some(source_index) = - layer_catalog.index_for_id(&source_unit.declaration_id()) - else { + let Some(file) = scan_files_by_path.get(&site.path) else { continue; }; - let source_id = layer_catalog.nodes[source_index].key.id.clone(); - let mut resolved_targets = outcome - .definitions - .into_iter() - .filter(|unit| unit.fq_name() == to_name) - .filter_map(|unit| { - canonical_graph_unit_for_id(&endpoints, &unit.declaration_id()) - }) - .collect::>(); - resolved_targets.sort_by_key(CodeUnit::declaration_id); - resolved_targets.dedup_by_key(|unit| unit.declaration_id()); - if let Some(target) = resolved_targets.first() { - point_pairs.insert((source_id.clone(), target.declaration_id())); + if !site.exact_targets.is_empty() { + let (start, end) = site.spans[0]; + let range = Range { + start_byte: start, + end_byte: end, + start_line: site.line, + end_line: site.line, + }; + if let Some(source_unit) = analyzer + .enclosing_code_unit(file, &range) + .filter(|unit| unit.fq_name() == target.from_name) + && let Some(source_index) = + layer_catalog.index_for_id(&source_unit.declaration_id()) + { + let source_id = layer_catalog.nodes[source_index].key.id.clone(); + let pairs = site + .exact_targets + .iter() + .filter(|candidate| { + candidate.fq_name() == target.to_name + && is_graph_declaration(candidate) + }) + .map(|candidate| { + if !endpoints.iter().any(|endpoint| { + endpoint.declaration_id() + == candidate.declaration_id() + }) { + endpoints.push(candidate.clone()); + } + candidate.clone() + }) + .map(|candidate| (source_id.clone(), candidate.declaration_id())) + .collect::>(); + if !pairs.is_empty() { + new_authoritative_sites.push(site_key.clone()); + new_exact_by_site.push((site_key, pairs)); + continue; + } + } } + new_pending_slow_path.push(( + file.clone(), + DeferredSlowPathSite { + site_key, + from_name: target.from_name.to_string(), + to_name: target.to_name.to_string(), + line: site.line, + spans: site.spans.clone(), + endpoints_snapshot: endpoints.clone(), + }, + )); } - if point_pairs.len() == 1 { - point_exact_by_site - .entry(site_key) - .or_default() - .extend(point_pairs); - } - } - if !structural_exact_loaded { - structural_exact_loaded = true; - let structural = ReferenceEngine::new() - .scan_file_edges_at_lines(analyzer, &structural_exact_sites); - if !structural.completeness.is_complete() { - incomplete.insert(( + + // This scan's `ReferenceEngine` never carries a real + // deadline (no `.with_cancellation` above), so the + // interruptible, per-candidate importer scan + // `references_to_edges` uses by default buys nothing + // here -- it only protects a caller that can actually be + // cancelled mid-scan. Passing the import-graph provider + // explicitly routes candidate discovery through the + // cached reverse-import-index path instead, which a + // workspace the size of a large monorepo otherwise + // re-scans from scratch for every ambiguous target + // (bifrost#15). + let exact = ReferenceEngine::new() + .with_file_filter(|file| admitted_files.contains(file)) + .references_to_edges_with_provider( + analyzer, + &endpoints, + Some(&crate::analyzer::usages::ImportGraphCandidateProvider::new()), + scan_files.len(), + crate::analyzer::usages::inverted_edges::MAX_CALLSITES + .saturating_mul(endpoints.len()), + None, + ); + let mut incomplete_reasons = Vec::new(); + if !exact.completeness.is_complete() { + incomplete_reasons.push(( "exact_reference_join_incomplete".to_string(), - "structural exact reference attribution was incomplete".to_string(), + format!( + "exact reference attribution was incomplete for ambiguous target {}", + target.to_name + ), )); } - for row in structural.edges { + let mut new_inverse_exact_by_site = UsageGraphExactSites::default(); + for row in exact.edges { let Some(source) = row.site.enclosing.as_ref() else { continue; }; @@ -3235,58 +3382,147 @@ pub fn usage_graph(analyzer: &dyn IAnalyzer, params: UsageGraphParams) -> UsageG source.fq_name(), row.target.fq_name(), ); - if authoritative_exact_sites.contains(&site_key) { + if authoritative_exact_sites_snapshot.contains(&site_key) { continue; } - structural_exact_by_site + new_inverse_exact_by_site .entry(site_key) .or_default() .insert((source.declaration_id(), row.target_id())); } - } - // The bounded semantic pass deliberately aggregates by its legacy - // graph key. When that key names multiple exact declarations, ask - // the same language plugin's target side to retain the overload - // identity for only this layer's admitted files. This is a rare - // ambiguity join, not a second unconditional workspace scan. - let admitted_files = scan_files.iter().cloned().collect::>(); - let exact = ReferenceEngine::new() - .with_file_filter(|file| admitted_files.contains(file)) - .references_to_edges( - analyzer, - &endpoints, - scan_files.len(), - crate::analyzer::usages::inverted_edges::MAX_CALLSITES - .saturating_mul(endpoints.len()), - None, - ); - if !exact.completeness.is_complete() { - incomplete.insert(( - "exact_reference_join_incomplete".to_string(), - format!( - "exact reference attribution was incomplete for ambiguous target {to_name}" - ), - )); - } - for row in exact.edges { - let Some(source) = row.site.enclosing.as_ref() else { + + AmbiguousTargetResult { + ecosystem: target.ecosystem, + to_name: target.to_name.to_string(), + endpoints, + new_authoritative_sites, + new_exact_by_site, + new_pending_slow_path, + new_inverse_exact_by_site, + incomplete_reasons, + } + }) + .collect() + }); + + let mut endpoints_by_target: HashMap<(UsageEcosystem, String), Vec> = + HashMap::default(); + for result in results { + for site_key in result.new_authoritative_sites { + authoritative_exact_sites.insert(site_key); + } + for (site_key, pairs) in result.new_exact_by_site { + exact_by_site.insert(site_key, pairs); + } + for (file, pending) in result.new_pending_slow_path { + pending_slow_path.entry(file).or_default().push(pending); + } + for (site_key, pairs) in result.new_inverse_exact_by_site { + inverse_exact_by_site + .entry(site_key) + .or_default() + .extend(pairs); + } + for reason in result.incomplete_reasons { + incomplete.insert(reason); + } + endpoints_by_target.insert((result.ecosystem, result.to_name), result.endpoints); + } + + // Batch every deferred slow-path site, one `resolve_definition_batch_with_source` + // call per file instead of one per edge (see the comment above + // `pending_slow_path`'s declaration). + for (file, pending_sites) in &pending_slow_path { + let Some(source) = analyzer.indexed_source(file) else { + continue; + }; + let mut all_requests = Vec::new(); + let mut request_counts = Vec::with_capacity(pending_sites.len()); + for pending in pending_sites { + let before = all_requests.len(); + all_requests.extend(pending.spans.iter().map(|(start, end)| { + crate::analyzer::usages::get_definition::DefinitionLookupRequest { + file: file.clone(), + line: None, + column: None, + start_byte: Some(*start), + end_byte: Some(*end), + } + })); + request_counts.push(all_requests.len() - before); + } + let all_outcomes = + crate::analyzer::usages::get_definition::resolve_definition_batch_with_source( + analyzer, + all_requests, + file.clone(), + source.into(), + ); + let mut offset = 0; + for (pending, count) in pending_sites.iter().zip(&request_counts) { + let outcomes_slice = &all_outcomes[offset..offset + count]; + offset += count; + let mut point_pairs = BTreeSet::new(); + for ((start, end), outcome) in pending.spans.iter().zip(outcomes_slice) { + if outcome.status + != crate::analyzer::usages::get_definition::DefinitionLookupStatus::Resolved + { + continue; + } + let range = Range { + start_byte: *start, + end_byte: *end, + start_line: pending.line, + end_line: pending.line, + }; + let Some(source_unit) = analyzer + .enclosing_code_unit(file, &range) + .filter(|unit| unit.fq_name() == pending.from_name) + else { continue; }; - let site_key = ( - rel_path_string(&row.site.file), - row.site.range.start_line, - source.fq_name(), - row.target.fq_name(), - ); - if authoritative_exact_sites.contains(&site_key) { + let Some(source_index) = + layer_catalog.index_for_id(&source_unit.declaration_id()) + else { continue; + }; + let source_id = layer_catalog.nodes[source_index].key.id.clone(); + let mut resolved_targets = outcome + .definitions + .iter() + .cloned() + .filter(|unit| unit.fq_name() == pending.to_name) + .filter_map(|unit| { + canonical_graph_unit_for_id( + &pending.endpoints_snapshot, + &unit.declaration_id(), + ) + }) + .collect::>(); + resolved_targets.sort_by_key(CodeUnit::declaration_id); + resolved_targets.dedup_by_key(|unit| unit.declaration_id()); + if let Some(target) = resolved_targets.first() { + point_pairs.insert((source_id.clone(), target.declaration_id())); } - inverse_exact_by_site - .entry(site_key) + } + if point_pairs.len() == 1 { + point_exact_by_site + .entry(pending.site_key.clone()) .or_default() - .insert((source.declaration_id(), row.target_id())); + .extend(point_pairs); } } + } + + for ((ecosystem, from_name, to_name), sites) in legacy_edges { + let endpoints = endpoints_by_target + .remove(&(ecosystem, to_name.clone())) + .unwrap_or_else(|| { + endpoints_by_name + .get(&(ecosystem, to_name.clone())) + .cloned() + .unwrap_or_default() + }); for site in sites { let site_key = ( site.path.clone(), @@ -5606,6 +5842,257 @@ mod tests { ); } + #[test] + fn go_import_infos_for_files_batches_instead_of_defaulting_to_none() { + let fixture = AnalyzerFixture::new_for_language( + Language::Go, + &[ + ("go.mod", "module example.com/repro\n"), + ("helpers/alpha.go", "package helpers\n\nfunc Alpha() {}\n"), + ( + "caller/main.go", + "package caller\n\nimport \"example.com/repro/helpers\"\n\nfunc Run() {\n\thelpers.Alpha()\n}\n", + ), + ], + ); + let analyzer = fixture.analyzer.analyzer(); + let files: Vec = analyzer + .analyzed_files() + .into_iter() + .filter(|file| { + let path = rel_path_string(file); + path == "helpers/alpha.go" || path == "caller/main.go" + }) + .collect(); + assert_eq!( + files.len(), + 2, + "the fixture's two Go files must both be analyzed" + ); + + let provider = analyzer + .import_analysis_provider() + .expect("GoAnalyzer must expose an ImportAnalysisProvider"); + + // Before this fix, Go fell through to the trait's `None` default here, + // forcing find_direct_importers_with_cancellation to call + // `import_info_of` once per file inside its per-candidate parallel + // loop instead of one batched store read (bifrost#15). + let batched = provider + .import_infos_for_files(&files) + .expect("Go must implement the batched import-facts read, not fall back to None"); + assert_eq!( + batched.len(), + 2, + "the batch must return an entry for every requested file" + ); + + let caller = files + .iter() + .find(|file| rel_path_string(file) == "caller/main.go") + .expect("caller/main.go must be in the fixture"); + let caller_imports = batched + .get(caller) + .expect("caller/main.go must have a batched entry"); + assert!( + caller_imports + .iter() + .any(|info| info.raw_snippet.contains("example.com/repro/helpers")), + "the batched import facts for caller/main.go must include its real import, got {caller_imports:?}" + ); + + let helpers = files + .iter() + .find(|file| rel_path_string(file) == "helpers/alpha.go") + .expect("helpers/alpha.go must be in the fixture"); + assert!( + batched + .get(helpers) + .expect("helpers/alpha.go must have a batched entry") + .is_empty(), + "helpers/alpha.go declares no imports of its own" + ); + } + + #[test] + fn ambiguous_edges_sharing_a_file_batch_one_resolve_definition_batch_with_source_call() { + let fixture = AnalyzerFixture::new_for_language( + Language::Go, + &[ + ("go.mod", "module example.com/repro\n"), + // Three declarations sharing one fq name each make the target + // genuinely ambiguous (`unique_graph_unit` returns None), + // which is what gates entry into the slow-path fallback. Three + // distinct ambiguous targets (rather than two) exercise the + // parallel per-target resolution with more than a pair of + // workers, so a merge bug that only shows up with >2 workers + // (e.g. clobbering rather than accumulating) is caught. + ("dup/a.go", "package dup\n\nfunc Widget() {}\n"), + ("dup/b.go", "package dup\n\nfunc Widget() {}\n"), + ("dup2/a.go", "package dup2\n\nfunc Gadget() {}\n"), + ("dup2/b.go", "package dup2\n\nfunc Gadget() {}\n"), + ("dup3/a.go", "package dup3\n\nfunc Sprocket() {}\n"), + ("dup3/b.go", "package dup3\n\nfunc Sprocket() {}\n"), + ( + "caller/main.go", + "package caller\n\nimport (\n\t\"example.com/repro/dup\"\n\t\"example.com/repro/dup2\"\n\t\"example.com/repro/dup3\"\n)\n\nfunc RunMany() {\n\tdup.Widget()\n\tdup.Widget()\n\tdup2.Gadget()\n\tdup2.Gadget()\n\tdup3.Sprocket()\n\tdup3.Sprocket()\n}\n", + ), + ], + ); + let analyzer = fixture.analyzer.analyzer(); + + crate::analyzer::usages::get_definition::reset_resolve_definition_batch_with_source_call_count_for_test(); + crate::analyzer::usages::candidates::reset_find_direct_importers_with_cancellation_call_count_for_test(); + let graph = usage_graph( + analyzer, + UsageGraphParams { + include_tests: false, + paths: None, + depth: 1, + }, + ); + let calls = + crate::analyzer::usages::get_definition::resolve_definition_batch_with_source_call_count_for_test(); + let importer_scan_calls = + crate::analyzer::usages::candidates::find_direct_importers_with_cancellation_call_count_for_test(); + + // Three ambiguous targets (Widget, Gadget, Sprocket) each called + // twice from the same file are six fallback-eligible sites; batched + // by file they must collapse into exactly one call, not one per site + // and not one per target, regardless of how many targets' resolution + // ran in parallel (bifrost#15). + assert_eq!( + calls, 1, + "six ambiguous-edge fallback sites sharing caller/main.go must batch into \ + exactly one resolve_definition_batch_with_source call, got {calls}" + ); + // usage_graph()'s ReferenceEngine never carries a real cancellation + // deadline, so its ambiguous-target candidate discovery must route + // through the cached reverse-import-index path (ImportGraphCandidateProvider) + // instead of the interruptible, uncached per-file importer scan that + // path exists to protect a caller with a real deadline (bifrost#15). + assert_eq!( + importer_scan_calls, 0, + "usage_graph's candidate discovery must not fall back to the uncached \ + per-candidate importer scan, got {importer_scan_calls} calls" + ); + // The graph is still allowed to omit an edge it genuinely cannot + // disambiguate; this test's job is the call-count assertion above, + // not asserting a specific resolved edge for an intentionally + // ambiguous target. + let _ = graph; + } + + #[test] + fn prefetch_definitions_reaches_the_go_analyzer_through_the_ianalyzer_trait() { + let fixture = AnalyzerFixture::new_for_language( + Language::Go, + &[ + ("go.mod", "module example.com/repro\n"), + ("helpers/alpha.go", "package helpers\n\nfunc Alpha() {}\n"), + ("helpers/beta.go", "package helpers\n\nfunc Beta() {}\n"), + ], + ); + let analyzer = fixture.analyzer.analyzer(); + let names = ["Alpha", "Beta"] + .map(|name| format!("example.com/repro/helpers.{name}")) + .to_vec(); + + let scope = std::sync::Arc::new(crate::analyzer::AnalyzerQueryContext::default()); + analyzer.begin_query(&scope); + analyzer + .test_hooks() + .reset_definition_candidates_query_count_for_test(); + analyzer + .test_hooks() + .reset_definition_prefetch_batch_count_for_test(); + + // usage_graph() only has `&dyn IAnalyzer`, so the batched prefetch it + // calls before resolving structural-exact sites must actually reach + // GoAnalyzer's inner analyzer through the trait, not silently no-op + // against the trait's own default (bifrost#15). + analyzer.prefetch_definitions(&names); + assert_eq!( + analyzer + .test_hooks() + .definition_prefetch_batch_count_for_test(), + 1, + "one batched prefetch call through the IAnalyzer trait must reach \ + GoAnalyzer's inner analyzer" + ); + + for name in &names { + assert_eq!( + analyzer.definitions(name).count(), + 1, + "{name} must resolve to its declaration after the trait-level prefetch" + ); + } + assert_eq!( + analyzer + .test_hooks() + .definition_candidates_query_count_for_test(), + 0, + "a name warmed by prefetch_definitions must not fall back to a point lookup" + ); + analyzer.end_query(&scope); + } + + #[test] + fn prefetch_fqn_in_language_resolves_many_names_in_one_relational_store_call() { + let fixture = AnalyzerFixture::new_for_language( + Language::Go, + &[ + ("go.mod", "module example.com/repro\n"), + ("helpers/alpha.go", "package helpers\n\nfunc Alpha() {}\n"), + ("helpers/beta.go", "package helpers\n\nfunc Beta() {}\n"), + ("helpers/gamma.go", "package helpers\n\nfunc Gamma() {}\n"), + ("helpers/delta.go", "package helpers\n\nfunc Delta() {}\n"), + ], + ); + let analyzer = fixture.analyzer.analyzer(); + let names = ["Alpha", "Beta", "Gamma", "Delta"] + .map(|name| format!("example.com/repro/helpers.{name}")) + .to_vec(); + let definitions = AnalyzerDefinitionLookup::new(analyzer, Language::None); + + analyzer + .test_hooks() + .reset_relational_definition_batch_call_count_for_test(); + definitions.prefetch_fqn_in_language(Language::Go, &names); + + // Four distinct cross-package names in one language: resolving them + // one at a time takes 8 round trips (an exact-name attempt plus an + // identifier-candidate fallback per name, since a package-qualified + // reference like this misses the exact-name store index). Batching + // both phases collapses that to one round trip per phase regardless + // of how many distinct names a `usage_graph` request resolves + // (bifrost#15). + assert_eq!( + analyzer + .test_hooks() + .relational_definition_batch_call_count_for_test(), + 2, + "prefetching four names in one language must batch both the exact-name attempt \ + and the identifier-candidate fallback into one round trip each" + ); + + for name in &names { + assert_eq!( + definitions.fqn_in_language(name, Language::Go).len(), + 1, + "{name} must resolve to its declaration after the prefetch populated the cache" + ); + } + assert_eq!( + analyzer + .test_hooks() + .relational_definition_batch_call_count_for_test(), + 2, + "a cache hit after the prefetch must not issue another store round trip" + ); + } + #[test] fn exact_location_selector_accepts_the_indexed_declaration_range_start() { let source = "package repro\n\ntype Error struct{}\n\nfunc (e *Error) Error() string { return \"\" }\n";