foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-22 23:34:47 -05:00
Code Issues Packages 1 Wiki Activity

fix(lsp): reduce deadlocks with in memory documents (#9259)

Browse source
This commit is contained in:
Kitson Kelly 2021-01-26 10:47:12 +11:00 committed by GitHub
parent 1f8b83ba1a
commit 2b4e0be43c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 28 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
cli/lsp/diagnostics.rs
View file

@ -85,13 +85,14 @@ pub async fn generate_lint_diagnostics(
tokio::task::spawn_blocking(move || { tokio::task::spawn_blocking(move || {
let mut diagnostic_list = Vec::new(); let mut diagnostic_list = Vec::new();
let documents = state_snapshot.documents.lock().unwrap(); for specifier in state_snapshot.documents.open_specifiers() {
for specifier in documents.open_specifiers() { let version = state_snapshot.documents.version(specifier);
let version = documents.version(specifier);
let current_version = diagnostic_collection.get_version(specifier); let current_version = diagnostic_collection.get_version(specifier);
if version != current_version { if version != current_version {
let media_type = MediaType::from(specifier); let media_type = MediaType::from(specifier);
if let Ok(Some(source_code)) = documents.content(specifier) { if let Ok(Some(source_code)) =
state_snapshot.documents.content(specifier)
{
if let Ok(references) = if let Ok(references) =
get_lint_references(specifier, &media_type, &source_code) get_lint_references(specifier, &media_type, &source_code)
{ {
@ -239,24 +240,20 @@ pub async fn generate_ts_diagnostics(
) -> Result<DiagnosticVec, AnyError> { ) -> Result<DiagnosticVec, AnyError> {
let mut diagnostics = Vec::new(); let mut diagnostics = Vec::new();
let mut specifiers = Vec::new(); let mut specifiers = Vec::new();
{ for specifier in state_snapshot.documents.open_specifiers() {
let documents = state_snapshot.documents.lock().unwrap(); let version = state_snapshot.documents.version(specifier);
for specifier in documents.open_specifiers() {
let version = documents.version(specifier);
let current_version = diagnostic_collection.get_version(specifier); let current_version = diagnostic_collection.get_version(specifier);
if version != current_version { if version != current_version {
specifiers.push(specifier.clone()); specifiers.push(specifier.clone());
} }
} }
}
if !specifiers.is_empty() { if !specifiers.is_empty() {
let req = tsc::RequestMethod::GetDiagnostics(specifiers); let req = tsc::RequestMethod::GetDiagnostics(specifiers);
let res = ts_server.request(state_snapshot.clone(), req).await?; let res = ts_server.request(state_snapshot.clone(), req).await?;
let ts_diagnostic_map: TsDiagnostics = serde_json::from_value(res)?; let ts_diagnostic_map: TsDiagnostics = serde_json::from_value(res)?;
for (specifier_str, ts_diagnostics) in ts_diagnostic_map.iter() { for (specifier_str, ts_diagnostics) in ts_diagnostic_map.iter() {
let specifier = ModuleSpecifier::resolve_url(specifier_str)?; let specifier = ModuleSpecifier::resolve_url(specifier_str)?;
let version = let version = state_snapshot.documents.version(&specifier);
state_snapshot.documents.lock().unwrap().version(&specifier);
diagnostics.push(( diagnostics.push((
specifier, specifier,
version, version,
@ -279,13 +276,12 @@ pub async fn generate_dependency_diagnostics(
} else { } else {
return Err(custom_error("Deadlock", "deadlock locking sources")); return Err(custom_error("Deadlock", "deadlock locking sources"));
}; };
let documents = state_snapshot.documents.lock().unwrap(); for specifier in state_snapshot.documents.open_specifiers() {
for specifier in documents.open_specifiers() { let version = state_snapshot.documents.version(specifier);
let version = documents.version(specifier);
let current_version = diagnostic_collection.get_version(specifier); let current_version = diagnostic_collection.get_version(specifier);
if version != current_version { if version != current_version {
let mut diagnostic_list = Vec::new(); let mut diagnostic_list = Vec::new();
if let Some(dependencies) = documents.dependencies(specifier) { if let Some(dependencies) = state_snapshot.documents.dependencies(specifier) {
for (_, dependency) in dependencies.iter() { for (_, dependency) in dependencies.iter() {
if let (Some(code), Some(range)) = ( if let (Some(code), Some(range)) = (
&dependency.maybe_code, &dependency.maybe_code,
@ -306,7 +302,7 @@ pub async fn generate_dependency_diagnostics(
}) })
} }
ResolvedDependency::Resolved(specifier) => { ResolvedDependency::Resolved(specifier) => {
if !(documents.contains(&specifier) || sources.contains(&specifier)) { if !(state_snapshot.documents.contains(&specifier) || sources.contains(&specifier)) {
let is_local = specifier.as_url().scheme() == "file"; let is_local = specifier.as_url().scheme() == "file";
diagnostic_list.push(lsp_types::Diagnostic { diagnostic_list.push(lsp_types::Diagnostic {
range: *range, range: *range,

4
cli/lsp/language_server.rs
View file

@ -58,7 +58,7 @@ pub struct LanguageServer {
#[derive(Debug, Clone, Default)] #[derive(Debug, Clone, Default)]
pub struct StateSnapshot { pub struct StateSnapshot {
pub assets: Arc<Mutex<HashMap<ModuleSpecifier, Option<AssetDocument>>>>, pub assets: Arc<Mutex<HashMap<ModuleSpecifier, Option<AssetDocument>>>>,
pub documents: Arc<Mutex<DocumentCache>>, pub documents: DocumentCache,
pub sources: Arc<Mutex<Sources>>, pub sources: Arc<Mutex<Sources>>,
} }
@ -289,7 +289,7 @@ impl LanguageServer {
pub fn snapshot(&self) -> StateSnapshot { pub fn snapshot(&self) -> StateSnapshot {
StateSnapshot { StateSnapshot {
assets: self.assets.clone(), assets: self.assets.clone(),
documents: self.documents.clone(), documents: self.documents.lock().unwrap().clone(),
sources: self.sources.clone(), sources: self.sources.clone(),
} }
} }

49
cli/lsp/tsc.rs
View file

@ -822,13 +822,7 @@ fn cache_snapshot(
.contains_key(&(specifier.clone().into(), version.clone().into())) .contains_key(&(specifier.clone().into(), version.clone().into()))
{ {
let s = ModuleSpecifier::resolve_url(&specifier)?; let s = ModuleSpecifier::resolve_url(&specifier)?;
let content = state let content = state.state_snapshot.documents.content(&s)?.unwrap();
.state_snapshot
.documents
.lock()
.unwrap()
.content(&s)?
.unwrap();
state state
.snapshots .snapshots
.insert((specifier.into(), version.into()), content); .insert((specifier.into(), version.into()), content);
@ -913,13 +907,7 @@ fn get_change_range(state: &mut State, args: Value) -> Result<Value, AnyError> {
fn get_length(state: &mut State, args: Value) -> Result<Value, AnyError> { fn get_length(state: &mut State, args: Value) -> Result<Value, AnyError> {
let v: SourceSnapshotArgs = serde_json::from_value(args)?; let v: SourceSnapshotArgs = serde_json::from_value(args)?;
let specifier = ModuleSpecifier::resolve_url(&v.specifier)?; let specifier = ModuleSpecifier::resolve_url(&v.specifier)?;
if state if state.state_snapshot.documents.contains(&specifier) {
.state_snapshot
.documents
.lock()
.unwrap()
.contains(&specifier)
{
cache_snapshot(state, v.specifier.clone(), v.version.clone())?; cache_snapshot(state, v.specifier.clone(), v.version.clone())?;
let content = state let content = state
.snapshots .snapshots
@ -944,13 +932,7 @@ struct GetTextArgs {
fn get_text(state: &mut State, args: Value) -> Result<Value, AnyError> { fn get_text(state: &mut State, args: Value) -> Result<Value, AnyError> {
let v: GetTextArgs = serde_json::from_value(args)?; let v: GetTextArgs = serde_json::from_value(args)?;
let specifier = ModuleSpecifier::resolve_url(&v.specifier)?; let specifier = ModuleSpecifier::resolve_url(&v.specifier)?;
let content = if state let content = if state.state_snapshot.documents.contains(&specifier) {
.state_snapshot
.documents
.lock()
.unwrap()
.contains(&specifier)
{
cache_snapshot(state, v.specifier.clone(), v.version.clone())?; cache_snapshot(state, v.specifier.clone(), v.version.clone())?;
state state
.snapshots .snapshots
@ -974,9 +956,10 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
return Err(custom_error("Deadlock", "deadlock locking sources")); return Err(custom_error("Deadlock", "deadlock locking sources"));
}; };
let documents = state.state_snapshot.documents.lock().unwrap(); if state.state_snapshot.documents.contains(&referrer) {
if documents.contains(&referrer) { if let Some(dependencies) =
if let Some(dependencies) = documents.dependencies(&referrer) { state.state_snapshot.documents.dependencies(&referrer)
{
for specifier in &v.specifiers { for specifier in &v.specifiers {
if specifier.starts_with("asset:///") { if specifier.starts_with("asset:///") {
resolved.push(Some(( resolved.push(Some((
@ -995,7 +978,7 @@ fn resolve(state: &mut State, args: Value) -> Result<Value, AnyError> {
if let ResolvedDependency::Resolved(resolved_specifier) = if let ResolvedDependency::Resolved(resolved_specifier) =
resolved_import resolved_import
{ {
if documents.contains(&resolved_specifier) if state.state_snapshot.documents.contains(&resolved_specifier)
|| sources.contains(&resolved_specifier) || sources.contains(&resolved_specifier)
{ {
let media_type = if let Some(media_type) = let media_type = if let Some(media_type) =
@ -1050,9 +1033,7 @@ fn respond(state: &mut State, args: Value) -> Result<Value, AnyError> {
} }
fn script_names(state: &mut State, _args: Value) -> Result<Value, AnyError> { fn script_names(state: &mut State, _args: Value) -> Result<Value, AnyError> {
let documents = state.state_snapshot.documents.lock().unwrap(); Ok(json!(state.state_snapshot.documents.open_specifiers()))
let script_names = documents.open_specifiers();
Ok(json!(script_names))
} }
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
@ -1064,13 +1045,7 @@ struct ScriptVersionArgs {
fn script_version(state: &mut State, args: Value) -> Result<Value, AnyError> { fn script_version(state: &mut State, args: Value) -> Result<Value, AnyError> {
let v: ScriptVersionArgs = serde_json::from_value(args)?; let v: ScriptVersionArgs = serde_json::from_value(args)?;
let specifier = ModuleSpecifier::resolve_url(&v.specifier)?; let specifier = ModuleSpecifier::resolve_url(&v.specifier)?;
if let Some(version) = state if let Some(version) = state.state_snapshot.documents.version(&specifier) {
.state_snapshot
.documents
.lock()
.unwrap()
.version(&specifier)
{
return Ok(json!(version.to_string())); return Ok(json!(version.to_string()));
} else { } else {
let mut sources = state.state_snapshot.sources.lock().unwrap(); let mut sources = state.state_snapshot.sources.lock().unwrap();
@ -1336,8 +1311,6 @@ pub fn request(
mod tests { mod tests {
use super::*; use super::*;
use crate::lsp::documents::DocumentCache; use crate::lsp::documents::DocumentCache;
use std::sync::Arc;
use std::sync::Mutex;
fn mock_state_snapshot(sources: Vec<(&str, &str, i32)>) -> StateSnapshot { fn mock_state_snapshot(sources: Vec<(&str, &str, i32)>) -> StateSnapshot {
let mut documents = DocumentCache::default(); let mut documents = DocumentCache::default();
@ -1348,7 +1321,7 @@ mod tests {
} }
StateSnapshot { StateSnapshot {
assets: Default::default(), assets: Default::default(),
documents: Arc::new(Mutex::new(documents)), documents,
sources: Default::default(), sources: Default::default(),
} }
} }