Skip to content

Commit 1d51068

Browse files
committed
Cache builtin docs and canonicalized resolutions in LSP hot path
1 parent 9000996 commit 1d51068

File tree

3 files changed

+38
-28
lines changed

3 files changed

+38
-28
lines changed

crates/pcb-zen-core/src/lang/eval.rs

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,10 @@ pub struct EvalContextConfig {
245245

246246
impl EvalContextConfig {
247247
/// Create a new root EvalContextConfig.
248-
pub fn new(file_provider: Arc<dyn FileProvider>, resolution: ResolutionResult) -> Self {
248+
///
249+
/// The resolution's `package_resolutions` keys should already be
250+
/// canonicalized (see [`EvalContext::new`] which handles this).
251+
pub fn new(file_provider: Arc<dyn FileProvider>, resolution: Arc<ResolutionResult>) -> Self {
249252
use std::sync::OnceLock;
250253
static BUILTIN_DOCS: OnceLock<Arc<HashMap<String, String>>> = OnceLock::new();
251254
let builtin_docs = BUILTIN_DOCS
@@ -259,25 +262,10 @@ impl EvalContextConfig {
259262
})
260263
.clone();
261264

262-
// Canonicalize package_resolutions keys to match canonicalized file paths during lookup.
263-
let canon_resolutions = resolution
264-
.package_resolutions
265-
.iter()
266-
.map(|(root, deps)| {
267-
let canon_root = file_provider
268-
.canonicalize(root)
269-
.unwrap_or_else(|_| root.clone());
270-
(canon_root, deps.clone())
271-
})
272-
.collect();
273-
274-
let mut resolution = resolution;
275-
resolution.package_resolutions = canon_resolutions;
276-
277265
Self {
278266
builtin_docs,
279267
file_provider,
280-
resolution: Arc::new(resolution),
268+
resolution,
281269
path_to_spec: Arc::new(RwLock::new(HashMap::new())),
282270
module_path: ModulePath::root(),
283271
load_chain: HashSet::new(),
@@ -848,8 +836,13 @@ fn json_value_to_heap_value<'v>(json: &serde_json::Value, heap: &'v Heap) -> Val
848836

849837
impl EvalContext {
850838
/// Create a new EvalContext with a fresh session.
839+
///
840+
/// Canonicalizes `package_resolutions` keys so that path lookups during
841+
/// evaluation match the canonicalized file paths used elsewhere.
851842
pub fn new(file_provider: Arc<dyn FileProvider>, resolution: ResolutionResult) -> Self {
852-
let config = EvalContextConfig::new(file_provider, resolution);
843+
let mut resolution = resolution;
844+
resolution.canonicalize_keys(&*file_provider);
845+
let config = EvalContextConfig::new(file_provider, Arc::new(resolution));
853846
EvalSession::default().create_context(config)
854847
}
855848

crates/pcb-zen-core/src/resolution.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,20 @@ impl ResolutionResult {
398398
}
399399
}
400400

401+
/// Canonicalize `package_resolutions` keys using the given file provider.
402+
pub fn canonicalize_keys(&mut self, file_provider: &dyn crate::FileProvider) {
403+
self.package_resolutions = self
404+
.package_resolutions
405+
.iter()
406+
.map(|(root, deps)| {
407+
let canon = file_provider
408+
.canonicalize(root)
409+
.unwrap_or_else(|_| root.clone());
410+
(canon, deps.clone())
411+
})
412+
.collect();
413+
}
414+
401415
/// Compute the transitive dependency closure for a package.
402416
pub fn package_closure(&self, package_url: &str) -> PackageClosure {
403417
let workspace_info = &self.workspace_info;

crates/pcb-zen/src/lsp/mod.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pub struct LspEvalContext {
3535
inner: EvalContext,
3636
builtin_docs: HashMap<LspUrl, String>,
3737
file_provider: Arc<dyn FileProvider>,
38-
resolution_cache: RwLock<HashMap<PathBuf, ResolutionResult>>,
38+
resolution_cache: RwLock<HashMap<PathBuf, Arc<ResolutionResult>>>,
3939
workspace_root_cache: RwLock<HashMap<PathBuf, PathBuf>>,
4040
open_files: Arc<RwLock<HashMap<PathBuf, String>>>,
4141
netlist_subscriptions: Arc<RwLock<HashMap<PathBuf, HashMap<String, JsonValue>>>>,
@@ -273,8 +273,8 @@ impl LspEvalContext {
273273
let uri = LspUrl::File(path_buf.to_path_buf());
274274
let maybe_contents = self.get_load_contents(&uri).ok().flatten();
275275

276-
let resolution = self.resolution_for(path_buf);
277-
let mut ctx = EvalContext::new(self.file_provider.clone(), resolution)
276+
let config = self.config_for(path_buf);
277+
let mut ctx = EvalContext::from_session_and_config(Default::default(), config)
278278
.set_source_path(path_buf.to_path_buf());
279279

280280
if let Some(contents) = maybe_contents {
@@ -343,26 +343,28 @@ impl LspEvalContext {
343343
workspace_root
344344
}
345345

346-
fn resolution_for(&self, file_path: &Path) -> ResolutionResult {
346+
/// Return the cached, canonicalized resolution for the workspace that owns `file_path`.
347+
fn resolution_for(&self, file_path: &Path) -> Arc<ResolutionResult> {
347348
let workspace_root = self.workspace_root_for(file_path);
348349
if let Some(cached) = self.resolution_cache.read().unwrap().get(&workspace_root) {
349350
return cached.clone();
350351
}
351352

352-
let resolution = crate::get_workspace_info(&self.file_provider, &workspace_root)
353+
let mut resolution = crate::get_workspace_info(&self.file_provider, &workspace_root)
353354
.and_then(|mut ws| crate::resolve_dependencies(&mut ws, false, false))
354355
.unwrap_or_else(|_| ResolutionResult::empty());
356+
resolution.canonicalize_keys(&*self.file_provider);
357+
let resolution = Arc::new(resolution);
355358
self.resolution_cache
356359
.write()
357360
.unwrap()
358361
.insert(workspace_root, resolution.clone());
359362
resolution
360363
}
361364

362-
/// Create an EvalContextConfig for the given file.
365+
/// Create a fresh EvalContextConfig for the given file.
363366
fn config_for(&self, file_path: &Path) -> EvalContextConfig {
364-
let resolution = self.resolution_for(file_path);
365-
EvalContextConfig::new(self.file_provider.clone(), resolution)
367+
EvalContextConfig::new(self.file_provider.clone(), self.resolution_for(file_path))
366368
.set_eager(self.inner.is_eager())
367369
}
368370

@@ -904,8 +906,9 @@ impl LspContext for LspEvalContext {
904906
let maybe_contents = self.get_load_contents(&params.uri).ok().flatten();
905907

906908
// Evaluate the module
907-
let resolution = self.resolution_for(path_buf);
908-
let ctx = EvalContext::new(self.file_provider.clone(), resolution);
909+
let config = self.config_for(path_buf);
910+
let ctx =
911+
EvalContext::from_session_and_config(Default::default(), config);
909912

910913
let eval_result = if let Some(contents) = maybe_contents {
911914
ctx.set_source_path(path_buf.clone())

0 commit comments

Comments
 (0)