Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

chore(rome_*):reduce parking_lot usage #2818

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 7 additions & 5 deletions crates/rome_cli/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{
};

use hdrhistogram::Histogram;
use parking_lot::{Mutex, RwLock};
use std::sync::{Mutex, RwLock};
use tracing::{span, subscriber::Interest, Level, Metadata, Subscriber};
use tracing_subscriber::{
layer::Context,
Expand Down Expand Up @@ -170,6 +170,7 @@ where

METRICS
.write()
.unwrap()
.insert(CallsiteKey(metadata), Mutex::new(entry));

Interest::always()
Expand Down Expand Up @@ -238,12 +239,12 @@ where

// Acquire a read lock on the metrics storage, access the metrics entry
// associated with this call site and acquire a write lock on it
let metrics = METRICS.read();
let metrics = METRICS.read().unwrap();
let entry = metrics
.get(&CallsiteKey(span.metadata()))
.expect("callsite not found, it should have been registered in register_callsite");

let mut entry = entry.lock();
let mut entry = entry.lock().unwrap();
timing.record(now, &mut entry);
}
}
Expand All @@ -259,8 +260,9 @@ pub fn init_metrics() {
pub fn print_metrics() {
let mut histograms: Vec<_> = METRICS
.write()
.unwrap()
.drain()
.flat_map(|(key, entry)| entry.into_inner().into_histograms(key.0.name()))
.flat_map(|(key, entry)| entry.into_inner().unwrap().into_histograms(key.0.name()))
.collect();

histograms.sort_unstable_by(|(a, _), (b, _)| a.cmp(b));
Expand Down Expand Up @@ -395,7 +397,7 @@ mod tests {
};

let entry = {
let mut metrics = METRICS.write();
let mut metrics = METRICS.write().unwrap();
metrics.remove(&CallsiteKey(key))
};

Expand Down
8 changes: 4 additions & 4 deletions crates/rome_js_formatter/tests/prettier_tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use parking_lot::{const_mutex, Mutex};
use std::sync::Mutex;
use rome_rowan::{TextRange, TextSize};
use similar::{utils::diff_lines, Algorithm, ChangeTag};
use std::{
Expand Down Expand Up @@ -316,7 +316,7 @@ struct DiffReport {
impl DiffReport {
fn get() -> &'static Self {
static REPORTER: DiffReport = DiffReport {
state: const_mutex(Vec::new()),
state: Mutex::new(Vec::new()),
};

// Use an atomic Once to register an exit callback the first time any
Expand Down Expand Up @@ -349,7 +349,7 @@ impl DiffReport {
rome_formatted_result: String,
prettier_formatted_result: String,
) {
self.state.lock().push(DiffReportItem {
self.state.lock().unwrap().push(DiffReportItem {
file_name,
rome_formatted_result,
prettier_formatted_result,
Expand Down Expand Up @@ -396,7 +396,7 @@ impl DiffReport {
}

fn report_prettier(&self, report_type: ReportType, report_filename: String) {
let mut state = self.state.lock();
let mut state = self.state.lock().unwrap();
state.sort_by_key(|DiffReportItem { file_name, .. }| *file_name);
let mut report_metric_data = PrettierCompatibilityMetricData::default();
let mut sum_of_single_compatibility_file = 0_f64;
Expand Down
1 change: 0 additions & 1 deletion crates/rome_lsp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ tokio = { version = "1.15.0", features = ["full" ] }
tracing = { version = "0.1.31", default-features = false, features = ["std", "max_level_trace", "release_max_level_warn"] }
tracing-tree = "0.2.0"
tracing-subscriber = "0.3.5"
parking_lot = "0.12.0"
futures = "0.3"

[dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_lsp/src/handlers/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub(crate) fn code_actions(
session: &Session,
params: CodeActionParams,
) -> Result<Option<CodeActionResponse>> {
let workspace_settings = session.config.read().get_workspace_settings();
let workspace_settings = session.config.read().unwrap().get_workspace_settings();
if !workspace_settings.analysis.enable_code_actions {
return Ok(Some(Vec::new()));
}
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_lsp/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ impl LanguageServer for LSPServer {
self.session
.client_capabilities
.write()
.unwrap()
.replace(params.capabilities);

let init = InitializeResult {
Expand All @@ -60,7 +61,7 @@ impl LanguageServer for LSPServer {
async fn initialized(&self, _: InitializedParams) {
self.session.fetch_client_configuration().await;

if self.session.config.read().get_workspace_settings().unstable {
if self.session.config.read().unwrap().get_workspace_settings().unstable {
rome_flags::set_unstable_flags(rome_flags::FeatureFlags::ALL);
}

Expand Down
16 changes: 10 additions & 6 deletions crates/rome_lsp/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::url_interner::UrlInterner;
use crate::utils;
use futures::stream::futures_unordered::FuturesUnordered;
use futures::StreamExt;
use parking_lot::RwLock;
use rome_analyze::RuleCategories;
use rome_diagnostics::file::FileId;
use rome_fs::RomePath;
Expand All @@ -16,6 +15,7 @@ use rome_service::workspace::UpdateSettingsParams;
use rome_service::RomeError;
use rome_service::Workspace;
use std::collections::HashMap;
use std::sync::RwLock;
use tower_lsp::lsp_types;
use tracing::{error, trace};

Expand Down Expand Up @@ -56,6 +56,7 @@ impl Session {
pub(crate) fn document(&self, url: &lsp_types::Url) -> Result<Document, RomeError> {
self.documents
.read()
.unwrap()
.get(url)
.cloned()
.ok_or(RomeError::NotFound)
Expand All @@ -65,18 +66,18 @@ impl Session {
///
/// Used by [`handlers::text_document] to synchronize documents with the client.
pub(crate) fn insert_document(&self, url: lsp_types::Url, document: Document) {
self.documents.write().insert(url, document);
self.documents.write().unwrap().insert(url, document);
}

/// Remove the [`Document`] matching the provided [`lsp_types::Url`]
pub(crate) fn remove_document(&self, url: &lsp_types::Url) {
self.documents.write().remove(url);
self.documents.write().unwrap().remove(url);
}

/// Return the unique [FileId] associated with the url for this [Session].
/// This will assign a new FileId if there isn't one for the provided url.
pub(crate) fn file_id(&self, url: lsp_types::Url) -> FileId {
self.url_interner.write().intern(url)
self.url_interner.write().unwrap().intern(url)
}

pub(crate) fn file_path(&self, url: &lsp_types::Url) -> RomePath {
Expand All @@ -91,7 +92,7 @@ impl Session {
let rome_path = self.file_path(&url);
let doc = self.document(&url)?;

let workspace_settings = self.config.read().get_workspace_settings();
let workspace_settings = self.config.read().unwrap().get_workspace_settings();

let diagnostics = if workspace_settings.analysis.enable_diagnostics {
let diagnostics = self.workspace.pull_diagnostics(PullDiagnosticsParams {
Expand Down Expand Up @@ -120,6 +121,7 @@ impl Session {
let mut futures: FuturesUnordered<_> = self
.documents
.read()
.unwrap()
.keys()
.map(|url| self.update_diagnostics(url.clone()))
.collect();
Expand All @@ -135,6 +137,7 @@ impl Session {
pub(crate) fn can_register_did_change_configuration(&self) -> bool {
self.client_capabilities
.read()
.unwrap()
.as_ref()
.and_then(|c| c.workspace.as_ref())
.and_then(|c| c.did_change_configuration)
Expand All @@ -146,6 +149,7 @@ impl Session {
pub(crate) fn diagnostics_enabled(&self) -> bool {
self.config
.read()
.unwrap()
.get_workspace_settings()
.analysis
.enable_diagnostics
Expand All @@ -162,7 +166,7 @@ impl Session {

if let Ok(configurations) = configurations {
configurations.into_iter().next().and_then(|configuration| {
let mut config = self.config.write();
let mut config = self.config.write().unwrap();

config
.set_workspace_settings(configuration)
Expand Down