From 3e54960215c24fd3f86a21a1e4aa2f7c2a0858e4 Mon Sep 17 00:00:00 2001 From: Joshua Batty Date: Thu, 25 Jul 2024 16:46:47 +0200 Subject: [PATCH] cache pid lock files to reduce IO operations (#6297) ## Description Implements a new `PidLockedFiles` type for efficient file locking. Before this we were do file IO on every did change key press event. We now cache the results for efficiency. --- sway-lsp/src/core/document.rs | 77 ++++++++++++++++++--------- sway-lsp/src/handlers/notification.rs | 16 +++--- sway-lsp/src/server.rs | 6 ++- sway-lsp/src/server_state.rs | 4 +- 4 files changed, 69 insertions(+), 34 deletions(-) diff --git a/sway-lsp/src/core/document.rs b/sway-lsp/src/core/document.rs index a5599190a67..14e89b6c3ce 100644 --- a/sway-lsp/src/core/document.rs +++ b/sway-lsp/src/core/document.rs @@ -1,4 +1,6 @@ #![allow(dead_code)] +use std::sync::Arc; + use crate::{ error::{DirectoryError, DocumentError, LanguageServerError}, utils::document, @@ -109,30 +111,6 @@ impl TextDocument { } } -/// Marks the specified file as "dirty" by creating a corresponding flag file. -/// -/// This function ensures the necessary directory structure exists before creating the flag file. -pub fn mark_file_as_dirty(uri: &Url) -> Result<(), LanguageServerError> { - let path = document::get_path_from_url(uri)?; - Ok(PidFileLocking::lsp(path) - .lock() - .map_err(|e| DirectoryError::LspLocksDirFailed(e.to_string()))?) -} - -/// Removes the corresponding flag file for the specified Url. -/// -/// If the flag file does not exist, this function will do nothing. -pub fn remove_dirty_flag(uri: &Url) -> Result<(), LanguageServerError> { - let path = document::get_path_from_url(uri)?; - let uri = uri.clone(); - Ok(PidFileLocking::lsp(path) - .release() - .map_err(|err| DocumentError::UnableToRemoveFile { - path: uri.path().to_string(), - err: err.to_string(), - })?) -} - #[derive(Debug)] struct EditText<'text> { start_index: usize, @@ -242,6 +220,57 @@ impl std::ops::Deref for Documents { } } +/// Manages process-based file locking for multiple files. +pub struct PidLockedFiles { + locks: DashMap>, +} + +impl Default for PidLockedFiles { + fn default() -> Self { + Self::new() + } +} + +impl PidLockedFiles { + pub fn new() -> Self { + Self { + locks: DashMap::new(), + } + } + + /// Marks the specified file as "dirty" by creating a corresponding flag file. + /// + /// This function ensures the necessary directory structure exists before creating the flag file. + /// If the file is already locked, this function will do nothing. This is to reduce the number of + /// unnecessary file IO operations. + pub fn mark_file_as_dirty(&self, uri: &Url) -> Result<(), LanguageServerError> { + if !self.locks.contains_key(uri) { + let path = document::get_path_from_url(uri)?; + let file_lock = Arc::new(PidFileLocking::lsp(path)); + file_lock + .lock() + .map_err(|e| DirectoryError::LspLocksDirFailed(e.to_string()))?; + self.locks.insert(uri.clone(), file_lock); + } + Ok(()) + } + + /// Removes the corresponding flag file for the specified Url. + /// + /// If the flag file does not exist, this function will do nothing. + pub fn remove_dirty_flag(&self, uri: &Url) -> Result<(), LanguageServerError> { + if let Some((uri, file_lock)) = self.locks.remove(uri) { + file_lock + .release() + .map_err(|err| DocumentError::UnableToRemoveFile { + path: uri.path().to_string(), + err: err.to_string(), + })?; + } + Ok(()) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/sway-lsp/src/handlers/notification.rs b/sway-lsp/src/handlers/notification.rs index 99cb0406bef..928262efb51 100644 --- a/sway-lsp/src/handlers/notification.rs +++ b/sway-lsp/src/handlers/notification.rs @@ -2,10 +2,7 @@ //! Protocol. This module specifically handles notification messages sent by the Client. use crate::{ - core::{ - document::{self, Documents}, - session::Session, - }, + core::{document::Documents, session::Session}, error::LanguageServerError, server_state::{CompilationContext, ServerState, TaskMessage}, }; @@ -89,7 +86,10 @@ pub async fn handle_did_change_text_document( state: &ServerState, params: DidChangeTextDocumentParams, ) -> Result<(), LanguageServerError> { - if let Err(err) = document::mark_file_as_dirty(¶ms.text_document.uri) { + if let Err(err) = state + .pid_locked_files + .mark_file_as_dirty(¶ms.text_document.uri) + { tracing::warn!("Failed to mark file as dirty: {}", err); } @@ -138,7 +138,9 @@ pub(crate) async fn handle_did_save_text_document( state: &ServerState, params: DidSaveTextDocumentParams, ) -> Result<(), LanguageServerError> { - document::remove_dirty_flag(¶ms.text_document.uri)?; + state + .pid_locked_files + .remove_dirty_flag(¶ms.text_document.uri)?; let (uri, session) = state .uri_and_session_from_workspace(¶ms.text_document.uri) .await?; @@ -159,7 +161,7 @@ pub(crate) async fn handle_did_change_watched_files( for event in params.changes { let (uri, _) = state.uri_and_session_from_workspace(&event.uri).await?; if let FileChangeType::DELETED = event.typ { - document::remove_dirty_flag(&event.uri)?; + state.pid_locked_files.remove_dirty_flag(&event.uri)?; let _ = state.documents.remove_document(&uri); } } diff --git a/sway-lsp/src/server.rs b/sway-lsp/src/server.rs index 34314c89381..17efc9876de 100644 --- a/sway-lsp/src/server.rs +++ b/sway-lsp/src/server.rs @@ -2,7 +2,6 @@ //! It provides an interface between the LSP protocol and the sway-lsp internals. use crate::{ - core::document, handlers::{notification, request}, lsp_ext::{MetricsParams, OnEnterParams, ShowAstParams, VisualizeParams}, server_state::ServerState, @@ -43,7 +42,10 @@ impl LanguageServer for ServerState { } async fn did_close(&self, params: DidCloseTextDocumentParams) { - if let Err(err) = document::remove_dirty_flag(¶ms.text_document.uri) { + if let Err(err) = self + .pid_locked_files + .remove_dirty_flag(¶ms.text_document.uri) + { tracing::error!("{}", err.to_string()); } } diff --git a/sway-lsp/src/server_state.rs b/sway-lsp/src/server_state.rs index 2f614e93072..3f7760b4544 100644 --- a/sway-lsp/src/server_state.rs +++ b/sway-lsp/src/server_state.rs @@ -3,7 +3,7 @@ use crate::{ config::{Config, GarbageCollectionConfig, Warnings}, core::{ - document::Documents, + document::{Documents, PidLockedFiles}, session::{self, Session}, }, error::{DirectoryError, DocumentError, LanguageServerError}, @@ -49,6 +49,7 @@ pub struct ServerState { pub(crate) cb_tx: Sender, pub(crate) cb_rx: Arc>, pub(crate) finished_compilation: Arc, + pub(crate) pid_locked_files: PidLockedFiles, last_compilation_state: Arc>, } @@ -66,6 +67,7 @@ impl Default for ServerState { cb_tx, cb_rx: Arc::new(cb_rx), finished_compilation: Arc::new(Notify::new()), + pid_locked_files: PidLockedFiles::new(), last_compilation_state: Arc::new(RwLock::new(LastCompilationState::Uninitialized)), }; // Spawn a new thread dedicated to handling compilation tasks