Skip to content

Commit cf31c84

Browse files
wagenetclaude
wagenet
and
claude
committed
perf(linter): reduce bulk-suppression overhead in concurrent path
Two targeted changes to the bulk-suppression machinery introduced in #19328 (oxlint-suppressions.json support). **Change 1 — Eliminate the startup clone in `concurrent_map()`** `SuppressionManager::concurrent_map()` previously constructed a `papaya::HashMap` by iterating the suppression file and cloning every key and value into it. Workers only ever *read* one entry per file, so the copy was pure overhead: ~10–15 ms sequential startup cost for 10k files / 20k suppressions. Fix: wrap `AllSuppressionsMap` in `Arc` at load time. `concurrent_map()` now calls `Arc::clone()` (a reference-count bump), making the hand-off to rayon workers zero-copy. Side effect: the `papaya` dependency can be removed from the worker path in `service/runtime.rs`; `papaya::HashMap::pin()` is replaced by a plain `FxHashMap::get`. The `&SuppressionFile::new(…)` dangling- reference pattern is also fixed while touching that code. **Change 2 — Reduce per-message `RuleName` allocations** `build_suppression_map` called `message.into()` twice per message (once for `get_mut`, once for `insert`). Replaced with `.entry().or_insert()` so the key is computed once. The filter closure in `suppress_lint_diagnostics` also called `message.into()` twice per message (two separate `.get()` calls). Replaced with `let key = RuleName::from(message)` so both lookups share one allocation. Net result: 1–2 heap allocations per message instead of 3–4. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent 99b850c commit cf31c84

File tree

3 files changed

+58
-72
lines changed

3 files changed

+58
-72
lines changed

‎crates/oxc_linter/src/service/runtime.rs‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -653,26 +653,18 @@ impl Runtime {
653653
return;
654654
}
655655

656-
let tmp_pin = concurrent_tracking_map.pin();
657-
658-
let suppression_file_check = {
659-
if ignore_suppression {
660-
None
661-
} else if let Ok(file_path) = path.strip_prefix(&self.cwd) {
662-
let filename = Filename::new(file_path);
663-
664-
let key_arc = Arc::from(filename);
665-
666-
let suppression_data = tmp_pin.get(&key_arc);
667-
668-
Some(&SuppressionFile::new(
669-
file_exists,
670-
self.linter.options.suppress_all,
671-
suppression_data,
672-
))
673-
} else {
674-
Some(&SuppressionFile::default())
675-
}
656+
let suppression_file_check = if ignore_suppression {
657+
None
658+
} else if let Ok(file_path) = path.strip_prefix(&self.cwd) {
659+
let filename = Filename::new(file_path);
660+
let suppression_data = concurrent_tracking_map.get(&filename);
661+
Some(SuppressionFile::new(
662+
file_exists,
663+
self.linter.options.suppress_all,
664+
suppression_data,
665+
))
666+
} else {
667+
Some(SuppressionFile::default())
676668
};
677669

678670
let (mut messages, disable_directives) =
@@ -716,15 +708,15 @@ impl Runtime {
716708
if let Some(suppression_file) = suppression_file_check {
717709
let (filtered_diagnostics, runtime_suppression_tracking) =
718710
SuppressionManager::suppress_lint_diagnostics(
719-
suppression_file,
711+
&suppression_file,
720712
messages,
721713
);
722714

723715
if let Some(suppression_detected) = runtime_suppression_tracking {
724716
let filename = Filename::new(path.strip_prefix(&self.cwd).unwrap());
725717

726718
let diffs = SuppressionManager::diff_filename(
727-
suppression_file,
719+
&suppression_file,
728720
&suppression_detected,
729721
&filename,
730722
);

‎crates/oxc_linter/src/suppression/mod.rs‎

Lines changed: 14 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
use std::{hash::BuildHasherDefault, path::Path, sync::Arc};
1+
use std::{path::Path, sync::Arc};
22

33
use oxc_diagnostics::OxcDiagnostic;
4-
use rustc_hash::{FxHashMap, FxHashSet, FxHasher};
4+
use rustc_hash::{FxHashMap, FxHashSet};
55

66
use crate::Message;
77

@@ -12,11 +12,7 @@ pub use tracking::{
1212
SuppressionTracking,
1313
};
1414

15-
type StaticSuppressionMap = papaya::HashMap<
16-
Arc<Filename>,
17-
FxHashMap<RuleName, DiagnosticCounts>,
18-
BuildHasherDefault<FxHasher>,
19-
>;
15+
type StaticSuppressionMap = Arc<FxHashMap<Filename, FxHashMap<RuleName, DiagnosticCounts>>>;
2016

2117
#[derive(Clone, Debug, PartialEq, Eq)]
2218
pub enum OxlintSuppressionFileAction {
@@ -95,26 +91,10 @@ impl SuppressionManager {
9591
}
9692

9793
pub fn concurrent_map(&self) -> StaticSuppressionMap {
98-
let concurrent_papaya = papaya::HashMap::builder()
99-
.hasher(BuildHasherDefault::default())
100-
.resize_mode(papaya::ResizeMode::Blocking)
101-
.build();
102-
103-
if !self.file_exists {
104-
return concurrent_papaya;
105-
}
106-
107-
let Some(ref file) = self.suppressions_by_file else {
108-
return concurrent_papaya;
109-
};
110-
111-
for file_key in file.suppressions().keys() {
112-
concurrent_papaya
113-
.pin()
114-
.insert(Arc::new(file_key.clone()), file.suppressions()[file_key].clone());
115-
}
116-
117-
concurrent_papaya
94+
self.suppressions_by_file
95+
.as_ref()
96+
.map(|f| Arc::clone(f.suppressions()))
97+
.unwrap_or_default()
11898
}
11999

120100
pub fn is_updating_file(&self) -> bool {
@@ -228,13 +208,11 @@ impl SuppressionManager {
228208
let mut suppression_tracking: FxHashMap<RuleName, DiagnosticCounts> =
229209
FxHashMap::default();
230210
for message in diagnostics {
231-
if let Some(violation_count) = suppression_tracking.get_mut(&message.into()) {
232-
violation_count.count += 1;
233-
} else {
234-
suppression_tracking.insert(message.into(), DiagnosticCounts { count: 1 });
235-
}
211+
suppression_tracking
212+
.entry(message.into())
213+
.or_insert(DiagnosticCounts { count: 0 })
214+
.count += 1;
236215
}
237-
238216
suppression_tracking
239217
};
240218

@@ -255,15 +233,13 @@ impl SuppressionManager {
255233
let diagnostics_filtered = lint_diagnostics
256234
.into_iter()
257235
.filter(|message| {
258-
let Some(count_file) = recorded_violations.get(&message.into()) else {
236+
let key = RuleName::from(message);
237+
let Some(count_file) = recorded_violations.get(&key) else {
259238
return true;
260239
};
261-
262-
let Some(count_runtime) = runtime_suppression_tracking.get(&message.into())
263-
else {
240+
let Some(count_runtime) = runtime_suppression_tracking.get(&key) else {
264241
return false;
265242
};
266-
267243
count_file.count < count_runtime.count
268244
})
269245
.collect();

‎crates/oxc_linter/src/suppression/tracking.rs‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::{ffi::OsStr, fs, path::Path};
1+
use std::{ffi::OsStr, fs, path::Path, sync::Arc};
22

33
use oxc_diagnostics::OxcDiagnostic;
44
use rustc_hash::FxHashMap;
@@ -64,18 +64,36 @@ impl std::fmt::Display for RuleName {
6464
}
6565

6666
type FileSuppressionsMap = FxHashMap<RuleName, DiagnosticCounts>;
67-
type AllSuppressionsMap = FxHashMap<Filename, FileSuppressionsMap>;
67+
type AllSuppressionsMap = Arc<FxHashMap<Filename, FileSuppressionsMap>>;
68+
69+
fn serialize_arc_map<S>(map: &AllSuppressionsMap, serializer: S) -> Result<S::Ok, S::Error>
70+
where
71+
S: serde::Serializer,
72+
{
73+
map.as_ref().serialize(serializer)
74+
}
75+
76+
fn deserialize_arc_map<'de, D>(deserializer: D) -> Result<AllSuppressionsMap, D::Error>
77+
where
78+
D: serde::Deserializer<'de>,
79+
{
80+
FxHashMap::<Filename, FileSuppressionsMap>::deserialize(deserializer).map(Arc::new)
81+
}
6882

6983
#[derive(Debug, Clone, Deserialize, Serialize)]
7084
#[serde(default)]
7185
pub struct SuppressionTracking {
7286
version: String,
87+
#[serde(
88+
deserialize_with = "deserialize_arc_map",
89+
serialize_with = "serialize_arc_map"
90+
)]
7391
suppressions: AllSuppressionsMap,
7492
}
7593

7694
impl Default for SuppressionTracking {
7795
fn default() -> Self {
78-
Self { version: "0.1.0".to_string(), suppressions: FxHashMap::default() }
96+
Self { version: "0.1.0".to_string(), suppressions: Arc::new(FxHashMap::default()) }
7997
}
8098
}
8199

@@ -120,27 +138,27 @@ impl SuppressionTracking {
120138
}
121139

122140
pub fn update(&mut self, diff: SuppressionDiff) {
141+
let map = Arc::make_mut(&mut self.suppressions);
123142
match diff {
124143
SuppressionDiff::Increased { file, rule, from: _, to }
125144
| SuppressionDiff::Decreased { file, rule, from: _, to } => {
126-
self.suppressions.get_mut(&file).unwrap().get_mut(&rule).unwrap().count = to;
145+
map.get_mut(&file).unwrap().get_mut(&rule).unwrap().count = to;
127146
}
128147
SuppressionDiff::PrunedRuled { file, rule } => {
129-
let file_map = &self.suppressions[&file];
130-
131-
if file_map.len() == 1 {
132-
self.suppressions.remove(&file);
148+
let file_map_len = map[&file].len();
149+
if file_map_len == 1 {
150+
map.remove(&file);
133151
} else {
134-
self.suppressions.get_mut(&file).unwrap().remove(&rule);
152+
map.get_mut(&file).unwrap().remove(&rule);
135153
}
136154
}
137155
SuppressionDiff::Appeared { file, rule, count } => {
138-
if let Some(file) = self.suppressions.get_mut(&file) {
139-
file.insert(rule, DiagnosticCounts { count });
156+
if let Some(file_entry) = map.get_mut(&file) {
157+
file_entry.insert(rule, DiagnosticCounts { count });
140158
} else {
141159
let mut file_diagnostic: FileSuppressionsMap = FxHashMap::default();
142160
file_diagnostic.insert(rule, DiagnosticCounts { count });
143-
self.suppressions.insert(file, file_diagnostic);
161+
map.insert(file, file_diagnostic);
144162
}
145163
}
146164
}

0 commit comments

Comments
 (0)