From 6e54645d783552cd69ed2cbe22a2ee6dd5ae9001 Mon Sep 17 00:00:00 2001 From: Sysix <3897725+Sysix@users.noreply.github.com> Date: Fri, 11 Jul 2025 14:50:22 +0000 Subject: [PATCH] refactor(language_server): store `LintService` instead of `Linter` (#12016) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I tried a basic usage with `htop` and @react-three/fiber as an import. The test setup should be included. Run `npm install` in the fixture folder. The results are looking not good. 😢 Would like to know more how I can memory check the server. My first guess is to modify it to a `lib` and then run a custom program with the methods of the `impl LanguageServer`. Current main: ![memory-main](https://github.com/user-attachments/assets/f76dea0d-b620-4c9e-b031-2781763dd54d) This PR: ![memory-pr12016](https://github.com/user-attachments/assets/7cf37b8a-64c7-404f-9e2e-acce4b48c2a5) `oxc_resolver.clear_cache()` as the first call on `Runtime.run_source()`: ![memory-clear-cache](https://github.com/user-attachments/assets/951c767f-1a34-4a50-b39b-5753f684789b) --- apps/oxlint/src/lint.rs | 6 +-- .../oxc_resolver_memory_leak/.gitignore | 1 + .../linter/oxc_resolver_memory_leak/index.tsx | 23 +++++++++ .../oxc_resolver_memory_leak/package.json | 7 +++ .../src/linter/isolated_lint_handler.rs | 48 +++++++++++-------- .../src/linter/server_linter.rs | 24 +++++----- crates/oxc_language_server/src/worker.rs | 5 +- crates/oxc_linter/src/service/mod.rs | 18 +++---- crates/oxc_linter/src/service/runtime.rs | 14 +++--- crates/oxc_linter/src/tester.rs | 3 +- 10 files changed, 97 insertions(+), 52 deletions(-) create mode 100644 crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/.gitignore create mode 100644 crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/index.tsx create mode 100644 crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/package.json diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 3b9ddb7d1c8b4..b141c6692d6de 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -303,15 +303,15 @@ impl Runner for LintRunner { // Spawn linting in another thread so diagnostics can be printed immediately from diagnostic_service.run. rayon::spawn(move || { - let mut lint_service = - LintService::new(&linter, allocator_pool, options).with_paths(paths); + let mut lint_service = LintService::new(linter, allocator_pool, options); + let _ = lint_service.with_paths(paths); // Use `RawTransferFileSystem` if `oxlint2` feature is enabled. // This reads the source text into start of allocator, instead of the end. #[cfg(all(feature = "oxlint2", not(feature = "disable_oxlint2")))] { use crate::raw_fs::RawTransferFileSystem; - lint_service = lint_service.with_file_system(Box::new(RawTransferFileSystem)); + let _ = lint_service.with_file_system(Box::new(RawTransferFileSystem)); } lint_service.run(&tx_error); diff --git a/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/.gitignore b/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/.gitignore new file mode 100644 index 0000000000000..3c3629e647f5d --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/.gitignore @@ -0,0 +1 @@ +node_modules diff --git a/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/index.tsx b/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/index.tsx new file mode 100644 index 0000000000000..98b27a8ca9d41 --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/index.tsx @@ -0,0 +1,23 @@ +import { Canvas } from '@react-three/fiber'; +// PR status +debugger; + +export function ThreeDMain() { + return ( + + + ) +} + diff --git a/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/package.json b/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/package.json new file mode 100644 index 0000000000000..f948d16fafdb1 --- /dev/null +++ b/crates/oxc_language_server/fixtures/linter/oxc_resolver_memory_leak/package.json @@ -0,0 +1,7 @@ +{ + "devPackages": {}, + "devDependencies": { + "@react-three/fiber": "^9.1.4", + "three": "^0.178.0" + } +} diff --git a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs index 0a2f06c85edb0..e00fc4193235d 100644 --- a/crates/oxc_language_server/src/linter/isolated_lint_handler.rs +++ b/crates/oxc_language_server/src/linter/isolated_lint_handler.rs @@ -12,8 +12,8 @@ use tower_lsp_server::{ use oxc_allocator::{Allocator, AllocatorPool}; use oxc_linter::{ - LINTABLE_EXTENSIONS, LintService, LintServiceOptions, Linter, MessageWithPosition, - loader::Loader, read_to_arena_str, + ConfigStore, LINTABLE_EXTENSIONS, LintOptions, LintService, LintServiceOptions, Linter, + MessageWithPosition, loader::Loader, read_to_arena_str, }; use oxc_linter::{RuntimeFileSystem, read_to_string}; @@ -29,8 +29,7 @@ pub struct IsolatedLintHandlerOptions { } pub struct IsolatedLintHandler { - linter: Linter, - options: IsolatedLintHandlerOptions, + service: LintService, } pub struct IsolatedLintHandlerFileSystem { @@ -63,11 +62,25 @@ impl RuntimeFileSystem for IsolatedLintHandlerFileSystem { } impl IsolatedLintHandler { - pub fn new(linter: Linter, options: IsolatedLintHandlerOptions) -> Self { - Self { linter, options } + pub fn new( + lint_options: LintOptions, + config_store: ConfigStore, + options: &IsolatedLintHandlerOptions, + ) -> Self { + let linter = Linter::new(lint_options, config_store); + let lint_service_options = LintServiceOptions::new(options.root_path.clone()) + .with_cross_module(options.use_cross_module); + + let service = LintService::new(linter, AllocatorPool::default(), lint_service_options); + + Self { service } } - pub fn run_single(&self, uri: &Uri, content: Option) -> Option> { + pub fn run_single( + &mut self, + uri: &Uri, + content: Option, + ) -> Option> { let path = uri.to_file_path()?; if !Self::should_lint_path(&path) { @@ -124,7 +137,7 @@ impl IsolatedLintHandler { } fn lint_path<'a>( - &self, + &mut self, allocator: &'a Allocator, path: &Path, source_text: Option, @@ -138,17 +151,14 @@ impl IsolatedLintHandler { debug!("lint {}", path.display()); - let lint_service_options = LintServiceOptions::new(self.options.root_path.clone()) - .with_cross_module(self.options.use_cross_module); - - let mut lint_service = - LintService::new(&self.linter, AllocatorPool::default(), lint_service_options) - .with_file_system(Box::new(IsolatedLintHandlerFileSystem::new( - path.to_path_buf(), - source_text, - ))) - .with_paths(vec![Arc::from(path.as_os_str())]); - let result = lint_service.run_source(allocator); + let result = self + .service + .with_file_system(Box::new(IsolatedLintHandlerFileSystem::new( + path.to_path_buf(), + source_text, + ))) + .with_paths(vec![Arc::from(path.as_os_str())]) + .run_source(allocator); Some(result) } diff --git a/crates/oxc_language_server/src/linter/server_linter.rs b/crates/oxc_language_server/src/linter/server_linter.rs index 94bee3b397237..08a7b40808dc0 100644 --- a/crates/oxc_language_server/src/linter/server_linter.rs +++ b/crates/oxc_language_server/src/linter/server_linter.rs @@ -5,11 +5,10 @@ use globset::Glob; use ignore::gitignore::Gitignore; use log::{debug, warn}; use rustc_hash::{FxBuildHasher, FxHashMap}; +use tokio::sync::Mutex; use tower_lsp_server::lsp_types::Uri; -use oxc_linter::{ - AllowWarnDeny, Config, ConfigStore, ConfigStoreBuilder, LintOptions, Linter, Oxlintrc, -}; +use oxc_linter::{AllowWarnDeny, Config, ConfigStore, ConfigStoreBuilder, LintOptions, Oxlintrc}; use tower_lsp_server::UriExt; use crate::linter::{ @@ -22,7 +21,7 @@ use crate::{ConcurrentHashMap, OXC_CONFIG_FILE, Options}; use super::config_walker::ConfigWalker; pub struct ServerLinter { - isolated_linter: Arc, + isolated_linter: Arc>, gitignore_glob: Vec, pub extended_paths: Vec, } @@ -85,15 +84,14 @@ impl ServerLinter { }, ); - let linter = Linter::new(lint_options, config_store); - let isolated_linter = IsolatedLintHandler::new( - linter, - IsolatedLintHandlerOptions { use_cross_module, root_path: root_path.to_path_buf() }, + lint_options, + config_store, + &IsolatedLintHandlerOptions { use_cross_module, root_path: root_path.to_path_buf() }, ); Self { - isolated_linter: Arc::new(isolated_linter), + isolated_linter: Arc::new(Mutex::new(isolated_linter)), gitignore_glob: Self::create_ignore_glob(&root_path, &oxlintrc), extended_paths, } @@ -200,12 +198,16 @@ impl ServerLinter { false } - pub fn run_single(&self, uri: &Uri, content: Option) -> Option> { + pub async fn run_single( + &self, + uri: &Uri, + content: Option, + ) -> Option> { if self.is_ignored(uri) { return None; } - self.isolated_linter.run_single(uri, content) + self.isolated_linter.lock().await.run_single(uri, content) } } diff --git a/crates/oxc_language_server/src/worker.rs b/crates/oxc_language_server/src/worker.rs index fb9f16093b5c9..9d7ed7530bd5a 100644 --- a/crates/oxc_language_server/src/worker.rs +++ b/crates/oxc_language_server/src/worker.rs @@ -159,7 +159,7 @@ impl WorkspaceWorker { return None; }; - server_linter.run_single(uri, content) + server_linter.run_single(uri, content).await } fn update_diagnostics(&self, uri: &Uri, diagnostics: &[DiagnosticReport]) { @@ -178,7 +178,8 @@ impl WorkspaceWorker { }; for uri in self.diagnostics_report_map.pin_owned().keys() { - if let Some(diagnostics) = server_linter.run_single(&Uri::from_str(uri).unwrap(), None) + if let Some(diagnostics) = + server_linter.run_single(&Uri::from_str(uri).unwrap(), None).await { self.diagnostics_report_map.pin().insert(uri.clone(), diagnostics.clone()); diagnostics_map.pin().insert(uri.clone(), diagnostics); diff --git a/crates/oxc_linter/src/service/mod.rs b/crates/oxc_linter/src/service/mod.rs index e3b5bf6ae0562..7f1bdf9b63a39 100644 --- a/crates/oxc_linter/src/service/mod.rs +++ b/crates/oxc_linter/src/service/mod.rs @@ -61,13 +61,13 @@ impl LintServiceOptions { } } -pub struct LintService<'l> { - runtime: Runtime<'l>, +pub struct LintService { + runtime: Runtime, } -impl<'l> LintService<'l> { +impl LintService { pub fn new( - linter: &'l Linter, + linter: Linter, allocator_pool: oxc_allocator::AllocatorPool, options: LintServiceOptions, ) -> Self { @@ -77,16 +77,16 @@ impl<'l> LintService<'l> { #[must_use] pub fn with_file_system( - mut self, + &mut self, file_system: Box, - ) -> Self { - self.runtime = self.runtime.with_file_system(file_system); + ) -> &mut Self { + self.runtime.with_file_system(file_system); self } #[must_use] - pub fn with_paths(mut self, paths: Vec>) -> Self { - self.runtime = self.runtime.with_paths(paths); + pub fn with_paths(&mut self, paths: Vec>) -> &mut Self { + self.runtime.with_paths(paths); self } diff --git a/crates/oxc_linter/src/service/runtime.rs b/crates/oxc_linter/src/service/runtime.rs index 5d0716767f383..3914cbcfa1544 100644 --- a/crates/oxc_linter/src/service/runtime.rs +++ b/crates/oxc_linter/src/service/runtime.rs @@ -34,11 +34,11 @@ use crate::{ #[cfg(feature = "language_server")] use crate::fixer::MessageWithPosition; -pub struct Runtime<'l> { +pub struct Runtime { cwd: Box, /// All paths to lint paths: IndexSet, FxBuildHasher>, - pub(super) linter: &'l Linter, + pub(super) linter: Linter, resolver: Option, pub(super) file_system: Box, @@ -176,9 +176,9 @@ impl RuntimeFileSystem for OsFileSystem { } } -impl<'l> Runtime<'l> { +impl Runtime { pub(super) fn new( - linter: &'l Linter, + linter: Linter, allocator_pool: AllocatorPool, options: LintServiceOptions, ) -> Self { @@ -196,14 +196,14 @@ impl<'l> Runtime<'l> { } pub fn with_file_system( - mut self, + &mut self, file_system: Box, - ) -> Self { + ) -> &Self { self.file_system = file_system; self } - pub fn with_paths(mut self, paths: Vec>) -> Self { + pub fn with_paths(&mut self, paths: Vec>) -> &Self { self.paths = paths.into_iter().collect(); self } diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs index 3ecbe2464a0ac..fea38019a2cd9 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/crates/oxc_linter/src/tester.rs @@ -538,7 +538,8 @@ impl Tester { let cwd = self.current_working_directory.clone(); let paths = vec![Arc::::from(path_to_lint.as_os_str())]; let options = LintServiceOptions::new(cwd).with_cross_module(self.plugins.has_import()); - let mut lint_service = LintService::new(&linter, AllocatorPool::default(), options) + let mut lint_service = LintService::new(linter, AllocatorPool::default(), options); + let _ = lint_service .with_file_system(Box::new(TesterFileSystem::new( path_to_lint, source_text.to_string(),