perf(linter): make all rules share a diagnostics vec (#5806)

## What This PR Does

Each time a lint rule is run on a file, it will now store diagnostics in a
shared vec instead of having its own list. This is done by moving `diagnostics`
from `LintContext` to `ContextHost`.

The motivating line of code can be found in `Linter::run` in
`oxc_linter/src/lib.rs#145`

```rust
rules.into_iter().flat_map(|(_, ctx)| ctx.into_message()).collect::<Vec<_>>()
```

Each `LintContext` allocates a new vec, and pushes diagnostics into it. After
all run-related methods have been run, a new vec is created and those
diagnostics are copied into it. This is `O(n+1)` allocations and `O(n)` copies,
not to mention that allocations for the original vecs cannot be re-used since
those vecs aren't dropped until after the new one is created.
This commit is contained in:
DonIsaac 2024-09-17 05:46:15 +00:00
parent e04841c6a1
commit 3725d5d44e
3 changed files with 35 additions and 22 deletions

View file

@ -5,7 +5,7 @@ use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc};
use crate::{
config::LintConfig,
disable_directives::{DisableDirectives, DisableDirectivesBuilder},
fixer::FixKind,
fixer::{FixKind, Message},
frameworks,
options::{LintOptions, LintPlugins},
utils, FrameworkFlags, RuleWithSeverity,
@ -33,9 +33,13 @@ use super::{plugin_name_to_prefix, LintContext};
/// - [Flyweight Pattern](https://en.wikipedia.org/wiki/Flyweight_pattern)
#[must_use]
#[non_exhaustive]
pub struct ContextHost<'a> {
pub(crate) struct ContextHost<'a> {
pub(super) semantic: Rc<Semantic<'a>>,
pub(super) disable_directives: DisableDirectives<'a>,
/// Diagnostics reported by the linter.
///
/// Contains diagnostics for all rules across a single file.
diagnostics: RefCell<Vec<Message<'a>>>,
/// Whether or not to apply code fixes during linting. Defaults to
/// [`FixKind::None`] (no fixing).
///
@ -56,6 +60,8 @@ impl<'a> ContextHost<'a> {
semantic: Rc<Semantic<'a>>,
options: LintOptions,
) -> Self {
const DIAGNOSTICS_INITIAL_CAPACITY: usize = 512;
// We should always check for `semantic.cfg()` being `Some` since we depend on it and it is
// unwrapped without any runtime checks after construction.
assert!(
@ -72,6 +78,7 @@ impl<'a> ContextHost<'a> {
Self {
semantic,
disable_directives,
diagnostics: RefCell::new(Vec::with_capacity(DIAGNOSTICS_INITIAL_CAPACITY)),
fix: options.fix,
file_path,
config: Arc::new(LintConfig::default()),
@ -82,7 +89,7 @@ impl<'a> ContextHost<'a> {
}
#[inline]
pub(crate) fn with_config(mut self, config: &Arc<LintConfig>) -> Self {
pub fn with_config(mut self, config: &Arc<LintConfig>) -> Self {
self.config = Arc::clone(config);
self
}
@ -108,14 +115,26 @@ impl<'a> ContextHost<'a> {
self.semantic.source_type()
}
pub(crate) fn spawn(self: Rc<Self>, rule: &RuleWithSeverity) -> LintContext<'a> {
const DIAGNOSTICS_INITIAL_CAPACITY: usize = 128;
#[inline]
pub(super) fn push_diagnostic(&self, diagnostic: Message<'a>) {
self.diagnostics.borrow_mut().push(diagnostic);
}
/// Take all diagnostics collected during linting.
pub fn take_diagnostics(&self) -> Vec<Message<'a>> {
// NOTE: diagnostics are only ever borrowed here and in push_diagnostic.
// The latter drops the reference as soon as the function returns, so
// this should never panic.
let mut messages = self.diagnostics.borrow_mut();
std::mem::take(&mut *messages)
}
pub fn spawn(self: Rc<Self>, rule: &RuleWithSeverity) -> LintContext<'a> {
let rule_name = rule.name();
let plugin_name = self.map_jest(rule.plugin_name(), rule_name);
LintContext {
parent: self,
diagnostics: RefCell::new(Vec::with_capacity(DIAGNOSTICS_INITIAL_CAPACITY)),
current_rule_name: rule_name,
current_plugin_name: plugin_name,
current_plugin_prefix: plugin_name_to_prefix(plugin_name),
@ -127,11 +146,8 @@ impl<'a> ContextHost<'a> {
#[cfg(test)]
pub(crate) fn spawn_for_test(self: Rc<Self>) -> LintContext<'a> {
const DIAGNOSTICS_INITIAL_CAPACITY: usize = 128;
LintContext {
parent: Rc::clone(&self),
diagnostics: RefCell::new(Vec::with_capacity(DIAGNOSTICS_INITIAL_CAPACITY)),
current_rule_name: "",
current_plugin_name: "eslint",
current_plugin_prefix: "eslint",
@ -175,3 +191,9 @@ impl<'a> ContextHost<'a> {
self
}
}
impl<'a> From<ContextHost<'a>> for Vec<Message<'a>> {
fn from(ctx_host: ContextHost<'a>) -> Self {
ctx_host.diagnostics.into_inner()
}
}

View file

@ -1,7 +1,7 @@
#![allow(rustdoc::private_intra_doc_links)] // useful for intellisense
mod host;
use std::{cell::RefCell, path::Path, rc::Rc};
use std::{path::Path, rc::Rc};
use oxc_cfg::ControlFlowGraph;
use oxc_diagnostics::{OxcDiagnostic, Severity};
@ -18,7 +18,7 @@ use crate::{
AllowWarnDeny, FrameworkFlags, OxlintEnv, OxlintGlobals, OxlintSettings,
};
pub use host::ContextHost;
pub(crate) use host::ContextHost;
#[derive(Clone)]
#[must_use]
@ -26,11 +26,6 @@ pub struct LintContext<'a> {
/// Shared context independent of the rule being linted.
parent: Rc<ContextHost<'a>>,
/// Diagnostics reported by the linter.
///
/// Contains diagnostics for all rules across all files.
diagnostics: RefCell<Vec<Message<'a>>>,
// states
current_plugin_name: &'static str,
current_plugin_prefix: &'static str,
@ -158,10 +153,6 @@ impl<'a> LintContext<'a> {
/* Diagnostics */
pub fn into_message(self) -> Vec<Message<'a>> {
self.diagnostics.into_inner()
}
fn add_diagnostic(&self, mut message: Message<'a>) {
if self.parent.disable_directives.contains(self.current_rule_name, message.span()) {
return;
@ -179,7 +170,7 @@ impl<'a> LintContext<'a> {
message.error = message.error.with_severity(self.severity);
}
self.diagnostics.borrow_mut().push(message);
self.parent.push_diagnostic(message);
}
/// Report a lint rule violation.

View file

@ -142,7 +142,7 @@ impl Linter {
}
}
rules.into_iter().flat_map(|(_, ctx)| ctx.into_message()).collect::<Vec<_>>()
ctx_host.take_diagnostics()
}
/// # Panics