From d45eee85ed9dcd46809ec4927dd3f6a4dd4af82f Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:44:45 +0800 Subject: [PATCH] perf: reuse semantic state for ToolService queries Fixes #356 --- .../wright-analyzer/src/canonical/service.rs | 111 ++++++++++++-- .../wright-analyzer/src/canonical/symbols.rs | 11 ++ crates/wright-bench/src/main.rs | 142 +++++++++++++++++- crates/wright-driver/src/service.rs | 41 +++-- crates/wright-driver/src/session.rs | 21 +++ crates/wright-driver/src/session/semantic.rs | 50 +++--- crates/wright-driver/tests/service.rs | 67 +++++++++ 7 files changed, 385 insertions(+), 58 deletions(-) diff --git a/crates/wright-analyzer/src/canonical/service.rs b/crates/wright-analyzer/src/canonical/service.rs index 21936c1..ee284f3 100644 --- a/crates/wright-analyzer/src/canonical/service.rs +++ b/crates/wright-analyzer/src/canonical/service.rs @@ -1,4 +1,4 @@ -use std::sync::Arc; +use std::{ops::Deref, sync::Arc}; use serde_json::{Value as JsonValue, json}; use workshop_rs::source::{FileId, Span}; @@ -7,14 +7,37 @@ use workshop_rs::{Event, Program}; use super::analysis::Finding; use super::cfg::cfg_response; use super::facts::persistent_objects; -use super::symbols::{ReferenceKind, RuleId, SemanticIndex, Symbol, SymbolId}; +use super::symbols::{Reference, ReferenceKind, RuleId, SemanticIndex, Symbol, SymbolId}; use crate::analysis::Boundedness; use crate::registry::{LintConfig, SkippedRule}; use crate::service::{ErrorInfo, Origin, Request, Response}; +#[derive(Clone)] +enum ProgramSource<'a> { + Borrowed(&'a Program), + Shared(Arc), +} + +impl ProgramSource<'_> { + fn as_ref(&self) -> &Program { + match self { + Self::Borrowed(program) => program, + Self::Shared(program) => program, + } + } +} + +impl Deref for ProgramSource<'_> { + type Target = Program; + + fn deref(&self) -> &Self::Target { + self.as_ref() + } +} + pub struct SemanticService<'a> { - program: &'a Program, - index: SemanticIndex, + program: ProgramSource<'a>, + index: Arc, findings: Vec, skipped: Vec, origin: Origin, @@ -62,8 +85,36 @@ impl<'a> SemanticService<'a> { config: LintConfig, registry: Arc, ) -> Self { - let index = SemanticIndex::build(program); - let report = registry.run_report(program, &config); + Self::with_program_and_origin_and_config_and_registry( + ProgramSource::Borrowed(program), + origin, + config, + registry, + ) + } + + pub fn with_shared_program( + program: Arc, + origin: Origin, + config: LintConfig, + registry: Arc, + ) -> SemanticService<'static> { + SemanticService::<'static>::with_program_and_origin_and_config_and_registry( + ProgramSource::Shared(program), + origin, + config, + registry, + ) + } + + fn with_program_and_origin_and_config_and_registry( + program: ProgramSource<'a>, + origin: Origin, + config: LintConfig, + registry: Arc, + ) -> Self { + let index = Arc::new(SemanticIndex::build(program.as_ref())); + let report = registry.run_report(program.as_ref(), &config); Self { program, index, @@ -74,6 +125,36 @@ impl<'a> SemanticService<'a> { registry, } } + + pub fn with_lint_config(&self, config: LintConfig) -> Self { + let report = self.registry.run_report(self.program.as_ref(), &config); + Self { + program: self.program.clone(), + index: Arc::clone(&self.index), + findings: report.findings, + skipped: report.skipped, + origin: self.origin.clone(), + config, + registry: Arc::clone(&self.registry), + } + } + + pub fn references_for_all_symbols(&self) -> JsonValue { + JsonValue::Array( + self.index + .references_for_all_symbols() + .into_iter() + .map(|references| { + json!( + references + .into_iter() + .map(reference_json) + .collect::>() + ) + }) + .collect(), + ) + } pub fn handle_json(&self, request_json: &str) -> String { let request: Request = match serde_json::from_str(request_json) { Ok(req) => req, @@ -93,16 +174,16 @@ impl<'a> SemanticService<'a> { pub fn handle(&self, request: &Request) -> Response { match request { Request::Version => Response::Ok { result: json!({"name": "wright-tool", "version": env!("CARGO_PKG_VERSION"), "capabilities": ["program", "rules", "symbols", "references", "usage", "cfg", "findings", "persistentObjects", "lintRules"]}) }, - Request::Program => Response::Ok { result: json!({"origin": self.origin, "files": file_count(self.program), "globalVariables": self.program.global_variables.len(), "playerVariables": self.program.player_variables.len(), "subroutines": self.program.subroutines.len(), "rules": self.program.rules.len(), "findings": self.findings.len()}) }, + Request::Program => Response::Ok { result: json!({"origin": self.origin, "files": file_count(self.program.as_ref()), "globalVariables": self.program.global_variables.len(), "playerVariables": self.program.player_variables.len(), "subroutines": self.program.subroutines.len(), "rules": self.program.rules.len(), "findings": self.findings.len()}) }, Request::ListRules => Response::Ok { result: json!(self.program.rules.iter().enumerate().map(|(id, rule)| json!({"id": id, "name": rule.name, "span": span_json(self.program.rule_span(id))})).collect::>()) }, Request::GetRule { rule } => self.rule(*rule as usize), Request::ListSymbols { kind } => Response::Ok { result: json!(self.index.symbols().filter(|symbol| kind.as_deref().is_none_or(|kind| symbol.kind.as_str() == kind)).map(symbol_json).collect::>()) }, Request::GetSymbol { symbol } => self.index.symbol(SymbolId::from_index(*symbol as usize)).map_or_else(|| self.error("invalid-id", format!("unknown symbol {symbol}")), |symbol| Response::Ok { result: symbol_json(symbol) }), - Request::FindReferences { symbol } => { let id = SymbolId::from_index(*symbol as usize); if self.index.symbol(id).is_none() { self.error("invalid-id", format!("unknown symbol {symbol}")) } else { Response::Ok { result: json!(self.index.references(id).into_iter().map(|reference| json!({"kind": reference_kind_name(reference.kind), "span": span_json(reference.span), "rule": reference.rule, "action": reference.action, "value": reference.value})).collect::>()) } } } + Request::FindReferences { symbol } => { let id = SymbolId::from_index(*symbol as usize); if self.index.symbol(id).is_none() { self.error("invalid-id", format!("unknown symbol {symbol}")) } else { Response::Ok { result: json!(self.index.references(id).into_iter().map(reference_json).collect::>()) } } } Request::GetUsage { symbol } => { let id = SymbolId::from_index(*symbol as usize); self.index.symbol(id).map_or_else(|| self.error("invalid-id", format!("unknown symbol {symbol}")), |data| { let usage = self.index.usage(id); Response::Ok { result: json!({"symbol": data.name, "reads": usage.reads, "writes": usage.writes, "calls": usage.calls, "rules": usage.rules}) } }) } - Request::GetCfg { rule } => cfg_response(self.program, *rule as usize), + Request::GetCfg { rule } => cfg_response(self.program.as_ref(), *rule as usize), Request::GetFindings => Response::Ok { result: json!(self.findings.iter().map(finding_json).collect::>()) }, - Request::GetPersistentObjects => Response::Ok { result: json!(persistent_objects(self.program)) }, + Request::GetPersistentObjects => Response::Ok { result: json!(persistent_objects(self.program.as_ref())) }, Request::LintRules => Response::Ok { result: lint_rules(&self.registry, &self.config, &self.skipped), } @@ -194,6 +275,16 @@ fn reference_kind_name(kind: ReferenceKind) -> &'static str { ReferenceKind::Call => "call", } } + +fn reference_json(reference: &Reference) -> JsonValue { + json!({ + "kind": reference_kind_name(reference.kind), + "span": span_json(reference.span), + "rule": reference.rule, + "action": reference.action, + "value": reference.value, + }) +} fn event_name(event: &Event) -> String { match event { Event::Subroutine(name) => format!("subroutine:{name}"), diff --git a/crates/wright-analyzer/src/canonical/symbols.rs b/crates/wright-analyzer/src/canonical/symbols.rs index edc081d..1970bb2 100644 --- a/crates/wright-analyzer/src/canonical/symbols.rs +++ b/crates/wright-analyzer/src/canonical/symbols.rs @@ -293,6 +293,17 @@ impl SemanticIndex { .filter(|reference| reference.symbol == symbol) .collect() } + pub(super) fn references_for_all_symbols(&self) -> Vec> { + let mut grouped = (0..self.symbols.len()) + .map(|_| Vec::new()) + .collect::>(); + for reference in &self.references { + if let Some(references) = grouped.get_mut(reference.symbol.index()) { + references.push(reference); + } + } + grouped + } pub fn usage(&self, symbol: SymbolId) -> UsageSummary { let mut usage = UsageSummary::default(); let mut rules = HashSet::new(); diff --git a/crates/wright-bench/src/main.rs b/crates/wright-bench/src/main.rs index 7c983b1..abfc9e2 100644 --- a/crates/wright-bench/src/main.rs +++ b/crates/wright-bench/src/main.rs @@ -6,12 +6,14 @@ use std::time::{Duration, Instant}; use wright_driver::CompilerSession; use wright_driver::Profile; -use wright_driver::config::{SessionConfig, SourceKind}; +use wright_driver::config::{InputSpec, SessionConfig, SourceKind}; +use wright_driver::service::{ToolRequest, ToolResponse, ToolService}; const BENCH_CONTRACT: &str = "wright-bench/v1"; const BENCH_CONFIG: &str = r#"{ "iterations": 5, "warmup": 1, + "semanticQueryRepeats": 5, "thresholds": { "maxMeanLatencyMs": 500.0, "maxEmittedBytes": 200000, @@ -76,9 +78,24 @@ struct Report { iterations: usize, thresholds: serde_json::Value, fixtures: Vec, + semantic_queries: SemanticQueryReport, summary: serde_json::Value, } +#[derive(serde::Serialize)] +struct SemanticQueryFixtureReport { + fixture: &'static str, + service_initialization_mean_ms: f64, + repeated_queries_mean_ms: f64, + loaded_service_mean_ms: f64, +} + +#[derive(serde::Serialize)] +struct SemanticQueryReport { + workload: serde_json::Value, + fixtures: Vec, +} + fn run() -> Result { let config: serde_json::Value = serde_json::from_str(BENCH_CONFIG).map_err(|e| format!("invalid bench config: {e}"))?; @@ -140,6 +157,12 @@ fn run() -> Result { reports.push(report); } + let semantic_queries = benchmark_semantic_queries( + iterations, + warmup, + config["semanticQueryRepeats"].as_u64().unwrap_or(5) as usize, + )?; + let rss_mb = peak_rss_mb(); if rss_mb > config["thresholds"]["maxRssMb"] @@ -156,6 +179,7 @@ fn run() -> Result { iterations, thresholds: config["thresholds"].clone(), fixtures: reports, + semantic_queries, summary: serde_json::json!({ "peakRssMb": rss_mb, "regressions": regressions }), }; let out = workspace_root().join("target"); @@ -169,6 +193,122 @@ fn run() -> Result { Ok(regressions.is_empty()) } +fn benchmark_semantic_queries( + iterations: usize, + warmup: usize, + repeats: usize, +) -> Result { + let root = workspace_root().join("tests/fixtures/workshop"); + let cases = benchmark_cases() + .into_iter() + .filter(|(id, _)| { + matches!( + *id, + "synthetic/declarations-rules" + | "synthetic/control-flow" + | "real-world/overpy-cake" + ) + }) + .collect::>(); + let mut fixtures = Vec::new(); + + for (id, path) in cases { + let source = std::fs::read_to_string(&path) + .map_err(|e| format!("cannot read semantic benchmark fixture '{id}': {e}"))?; + for _ in 0..warmup { + semantic_query_trial(&source, id, &root, repeats)?; + } + + let mut initialization = Vec::with_capacity(iterations); + let mut queries = Vec::with_capacity(iterations); + for _ in 0..iterations { + let (service_initialization, repeated_queries) = + semantic_query_trial(&source, id, &root, repeats)?; + initialization.push(service_initialization); + queries.push(repeated_queries); + } + let initialization_ms = mean(&initialization) * 1000.0; + let queries_ms = mean(&queries) * 1000.0; + fixtures.push(SemanticQueryFixtureReport { + fixture: id, + service_initialization_mean_ms: initialization_ms, + repeated_queries_mean_ms: queries_ms, + loaded_service_mean_ms: initialization_ms + queries_ms, + }); + } + + Ok(SemanticQueryReport { + workload: serde_json::json!({ + "fixtures": fixtures.iter().map(|fixture| fixture.fixture).collect::>(), + "requests": ["rules", "symbols", "references(symbol:0)", "usage(symbol:0)", "cfg(rule:0)", "findings", "persistentObjects", "lint", "lintRules", "inspect"], + "repeatsPerFixturePerIteration": repeats, + "timing": "CompilerSession::load is completed before timing; initialization measures ToolService::new, repeatedQueries measures the listed request sequence, and loadedService is their sum.", + }), + fixtures, + }) +} + +fn semantic_query_trial( + source: &str, + fixture: &str, + root: &Path, + repeats: usize, +) -> Result<(Duration, Duration), String> { + let safe_name = fixture.replace('/', "-"); + let input_dir = workspace_root().join("target/wright-bench-inputs"); + std::fs::create_dir_all(&input_dir).map_err(|e| e.to_string())?; + let path = input_dir.join(format!( + "wright-bench-{}-semantic-{safe_name}.ws", + std::process::id() + )); + std::fs::write(&path, source).map_err(|e| e.to_string())?; + let result = (|| { + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(path.clone()), + kind: SourceKind::Workshop, + root: Some(root.to_path_buf()), + profile: Profile::Compat, + ..SessionConfig::default() + }) + .map_err(|e| e.message)?; + session.load().map_err(|e| e.message)?; + + let start = Instant::now(); + let mut service = ToolService::new(&mut session).map_err(|e| e.message)?; + let initialization = start.elapsed(); + + let requests = [ + ToolRequest::Rules, + ToolRequest::Symbols { kind: None }, + ToolRequest::References { symbol: 0 }, + ToolRequest::Usage { symbol: 0 }, + ToolRequest::Cfg { rule: 0 }, + ToolRequest::Findings, + ToolRequest::PersistentObjects, + ToolRequest::Lint, + ToolRequest::LintRules, + ]; + let start = Instant::now(); + for _ in 0..repeats { + for request in &requests { + if let ToolResponse::Error { error } = service.handle(request) { + return Err(format!("{fixture}: {error:?}")); + } + } + let inspection = service.inspect(); + if !inspection.ok { + return Err(format!( + "{fixture}: inspect failed: {:?}", + inspection.diagnostics + )); + } + } + Ok((initialization, start.elapsed())) + })(); + let _ = std::fs::remove_file(&path); + result +} + fn compile( source: &str, fixture: &str, diff --git a/crates/wright-driver/src/service.rs b/crates/wright-driver/src/service.rs index 6bd060e..0ecfdb2 100644 --- a/crates/wright-driver/src/service.rs +++ b/crates/wright-driver/src/service.rs @@ -153,13 +153,24 @@ pub struct Capabilities { pub struct ToolService<'a> { session: &'a mut CompilerSession, loaded: Loaded, + semantic: SemanticService<'static>, + lint_semantic: Option>, } impl<'a> ToolService<'a> { /// Build the service over a session, loading the program eagerly. pub fn new(session: &'a mut CompilerSession) -> Result, Diagnostic> { let loaded = session.load()?; - Ok(ToolService { session, loaded }) + let semantic = + session.shared_service_with(&loaded, wright_analyzer::registry::LintConfig::default()); + let lint_semantic = (!session.config.lint.rules.is_empty()) + .then(|| semantic.with_lint_config(session.config.lint.clone())); + Ok(ToolService { + session, + loaded, + semantic, + lint_semantic, + }) } /// The loaded program snapshot (origin, input identity, canonical program). @@ -230,8 +241,7 @@ impl<'a> ToolService<'a> { ToolRequest::Findings => self.findings(), ToolRequest::PersistentObjects => self.persistent_objects(), ToolRequest::Lint => self.lint(), - ToolRequest::LintRules => self - .semantic_query_with_config(Request::LintRules, self.session.config.lint.clone()), + ToolRequest::LintRules => self.configured_semantic().handle(&Request::LintRules), ToolRequest::CallGraph => self.ok(self.call_graph()), ToolRequest::CostEstimate => self.ok(self.cost_estimate()), ToolRequest::TargetMetadata => self.ok(self.target_metadata()), @@ -312,7 +322,8 @@ impl<'a> ToolService<'a> { /// Inspect through the shared session pipeline. pub fn inspect(&mut self) -> Envelope { - self.session.inspect() + self.session + .inspect_loaded(self.loaded.clone(), &self.semantic) } /// Spawn the LPP provider client for `language_id` through the session's @@ -391,32 +402,18 @@ impl<'a> ToolService<'a> { /// Run one semantic query over the loaded program. fn semantic_query(&self, request: Request) -> ToolResponse { - self.semantic_query_with_config(request, wright_analyzer::registry::LintConfig::default()) + self.semantic.handle(&request) } - /// Run one semantic query over the loaded program with an explicit lint - /// configuration. - fn semantic_query_with_config( - &self, - request: Request, - config: wright_analyzer::registry::LintConfig, - ) -> ToolResponse { - self.semantic_service(config).handle(&request) - } - - fn semantic_service( - &self, - config: wright_analyzer::registry::LintConfig, - ) -> SemanticService<'_> { - self.session.service_with(&self.loaded, config) + fn configured_semantic(&self) -> &SemanticService<'static> { + self.lint_semantic.as_ref().unwrap_or(&self.semantic) } /// `lint`: rule metadata, effective configuration, and findings over the /// loaded program through the same semantic-service path as the CLI /// `lint` workflow (no duplicated rule execution, #98). fn lint(&self) -> ToolResponse { - let config = self.session.config.lint.clone(); - let service = self.semantic_service(config); + let service = self.configured_semantic(); let lint_rules = match service.handle(&Request::LintRules) { Response::Ok { result } => result, Response::Error { .. } => serde_json::json!({}), diff --git a/crates/wright-driver/src/session.rs b/crates/wright-driver/src/session.rs index b3f6d92..7f801aa 100644 --- a/crates/wright-driver/src/session.rs +++ b/crates/wright-driver/src/session.rs @@ -680,6 +680,27 @@ impl CompilerSession { ) } + pub(crate) fn shared_service_with( + &self, + loaded: &Loaded, + config: LintConfig, + ) -> SemanticService<'static> { + let origin = ServiceOrigin { + kind: if loaded.provenance == Provenance::Unmapped { + "provider-artifact".to_string() + } else { + loaded.origin.kind.clone() + }, + locale: loaded.origin.locale.clone(), + }; + SemanticService::with_shared_program( + Arc::clone(&loaded.program), + origin, + config, + Arc::clone(&self.lint_registry), + ) + } + /// Structural validation permits source-preserving Workshop fallbacks. /// Surface those nodes as blocking semantic diagnostics before presenting /// check/lint output as definitive. The catalog remains owned by diff --git a/crates/wright-driver/src/session/semantic.rs b/crates/wright-driver/src/session/semantic.rs index 8f38901..47a01d9 100644 --- a/crates/wright-driver/src/session/semantic.rs +++ b/crates/wright-driver/src/session/semantic.rs @@ -90,6 +90,15 @@ fn semantic_facts(service: &SemanticService<'_>) -> serde_json::Value { }) } +fn inspect_result(service: &SemanticService<'_>) -> InspectResult { + InspectResult { + program: service_response(service, &Request::Program), + rules: service_response(service, &Request::ListRules), + symbols: service_response(service, &Request::ListSymbols { kind: None }), + references: service.references_for_all_symbols(), + } +} + /// Add the resolved `path` to every semantic `span` in a JSON result. /// /// File 0 is the main input and resolves root-relative to the include root @@ -182,31 +191,22 @@ impl CompilerSession { |session, loaded| { let service = session.service(&loaded); session.progress(ProgressEvent::new(ProgressPhase::SemanticAnalysis)); - let program = service_response(&service, &Request::Program); - let rules = service_response(&service, &Request::ListRules); - let symbols = service_response(&service, &Request::ListSymbols { kind: None }); - let references = serde_json::Value::Array( - symbols - .as_array() - .map(|list| { - list.iter() - .filter_map(|s| s.get("id").and_then(serde_json::Value::as_u64)) - .map(|id| { - service_response( - &service, - &Request::FindReferences { symbol: id as u32 }, - ) - }) - .collect() - }) - .unwrap_or_default(), - ); - InspectResult { - program, - rules, - symbols, - references, - } + inspect_result(&service) + }, + ) + } + + pub(crate) fn inspect_loaded( + &mut self, + loaded: Loaded, + service: &SemanticService<'_>, + ) -> Envelope { + self.with_loaded( + "inspect", + |_| Ok(loaded), + |session, _loaded| { + session.progress(ProgressEvent::new(ProgressPhase::SemanticAnalysis)); + inspect_result(service) }, ) } diff --git a/crates/wright-driver/tests/service.rs b/crates/wright-driver/tests/service.rs index 637c6f2..fefc0fc 100644 --- a/crates/wright-driver/tests/service.rs +++ b/crates/wright-driver/tests/service.rs @@ -34,6 +34,73 @@ fn tool_service_queries_canonical_workshop() { } } +#[test] +fn tool_service_inspect_matches_session_inspect() { + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(workshop_path()), + kind: SourceKind::Workshop, + ..SessionConfig::default() + }) + .unwrap(); + let expected = session.inspect(); + let mut service = ToolService::new(&mut session).unwrap(); + let actual = service.inspect(); + + assert_eq!(actual.ok, expected.ok); + assert_eq!( + serde_json::to_value(actual.result).unwrap(), + serde_json::to_value(expected.result).unwrap() + ); +} + +#[test] +fn tool_service_lint_queries_keep_the_session_configuration() { + let mut lint = wright_driver::config::LintConfig::default(); + assert!(lint.set_severity_by_name("min-wait-loop", "error")); + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(workshop_path()), + kind: SourceKind::Workshop, + lint, + ..SessionConfig::default() + }) + .unwrap(); + let service = ToolService::new(&mut session).unwrap(); + + let lint_rules = match service.handle(&ToolRequest::LintRules) { + ToolResponse::Ok { result } => result, + ToolResponse::Error { error } => panic!("lintRules failed: {error:?}"), + }; + assert_eq!( + lint_rules["config"]["rules"]["min-wait-loop"]["severity"], + "error" + ); + + let lint = match service.handle(&ToolRequest::Lint) { + ToolResponse::Ok { result } => result, + ToolResponse::Error { error } => panic!("lint failed: {error:?}"), + }; + assert_eq!(lint["config"], lint_rules["config"]); + let configured_finding = lint["findings"] + .as_array() + .unwrap() + .iter() + .find(|finding| finding["code"] == "min-wait-loop") + .expect("the fixture triggers min-wait-loop"); + assert_eq!(configured_finding["severity"], "error"); + + let default_findings = match service.handle(&ToolRequest::Findings) { + ToolResponse::Ok { result } => result, + ToolResponse::Error { error } => panic!("findings failed: {error:?}"), + }; + let default_finding = default_findings + .as_array() + .unwrap() + .iter() + .find(|finding| finding["code"] == "min-wait-loop") + .expect("the fixture triggers min-wait-loop"); + assert_eq!(default_finding["severity"], "warning"); +} + #[test] fn tool_service_keeps_provider_refusals_structured() { let path = workspace_root().join("tests/fixtures/opy/basic-rule.opy");