Skip to content

Commit 8387bac

Browse files
committedOct 22, 2024·
perf(linter): apply small file optimization, up to 30% faster (#6600)
Theory: iterating over the rules three times has slightly worse cache locality, because the prior iterations have pushed `rule` out of the cache by the time we iterate over it again. By iterating over each rule only once, we improve cache performance (hopefully). We also don't need to collect rules to a Vec, so it saves some CPU/memory there too. In practice: the behavior here actually depends on the number of AST nodes that are in the program. If the number of nodes is large, then it's better to iterate over the nodes only once and iterate the rules multiple times. But if the number of nodes is small, then it's better to iterate over nodes multiple times and only iterate over the rules once. See this comment for more context: #6600 (comment), as well as the comment inside the PR: https://github.com/oxc-project/oxc/pull/6600/files#diff-207225884c5e031ffd802bb99e4fbacbd8364b1343a1cec5485bf50f29186300R131-R143. In practice, this can make linting a file 1-45% faster, depending on the size of the file, number of AST nodes, number of files, CPU cache size, etc. To accommodate large and small files better, we have an explicit threshold of 200,000 AST nodes, which is an arbitrary number picked based on some benchmarks on my laptop. For large files, the linter behavior doesn't change. For small files, we switch to iterating over nodes in the inner loop and iterating over rules once in the outer loop.
1 parent 619d06f commit 8387bac

File tree

1 file changed

+60
-16
lines changed

1 file changed

+60
-16
lines changed
 

‎crates/oxc_linter/src/lib.rs

+60-16
Original file line numberDiff line numberDiff line change
@@ -121,30 +121,74 @@ impl Linter {
121121
.rules
122122
.iter()
123123
.filter(|rule| rule.should_run(&ctx_host))
124-
.map(|rule| (rule, Rc::clone(&ctx_host).spawn(rule)))
125-
.collect::<Vec<_>>();
126-
127-
for (rule, ctx) in &rules {
128-
rule.run_once(ctx);
129-
}
124+
.map(|rule| (rule, Rc::clone(&ctx_host).spawn(rule)));
130125

131126
let semantic = ctx_host.semantic();
132-
for symbol in semantic.symbols().symbol_ids() {
127+
128+
let should_run_on_jest_node =
129+
self.options.plugins.has_test() && ctx_host.frameworks().is_test();
130+
131+
// IMPORTANT: We have two branches here for performance reasons:
132+
//
133+
// 1) Branch where we iterate over each node, then each rule
134+
// 2) Branch where we iterate over each rule, then each node
135+
//
136+
// When the number of nodes is relatively small, most of them can fit
137+
// in the cache and we can save iterating over the rules multiple times.
138+
// But for large files, the number of nodes can be so large that it
139+
// starts to not fit into the cache and pushes out other data, like the rules.
140+
// So we end up thrashing the cache with each rule iteration. In this case,
141+
// it's better to put rules in the inner loop, as the rules data is smaller
142+
// and is more likely to fit in the cache.
143+
//
144+
// The threshold here is chosen to balance between performance improvement
145+
// from not iterating over rules multiple times, but also ensuring that we
146+
// don't thrash the cache too much. Feel free to tweak based on benchmarking.
147+
//
148+
// See https://github.com/oxc-project/oxc/pull/6600 for more context.
149+
if semantic.stats().nodes > 200_000 {
150+
// Collect rules into a Vec so that we can iterate over the rules multiple times
151+
let rules = rules.collect::<Vec<_>>();
152+
133153
for (rule, ctx) in &rules {
134-
rule.run_on_symbol(symbol, ctx);
154+
rule.run_once(ctx);
135155
}
136-
}
137156

138-
for node in semantic.nodes() {
139-
for (rule, ctx) in &rules {
140-
rule.run(node, ctx);
157+
for symbol in semantic.symbols().symbol_ids() {
158+
for (rule, ctx) in &rules {
159+
rule.run_on_symbol(symbol, ctx);
160+
}
141161
}
142-
}
143162

144-
if ctx_host.frameworks().is_test() && self.options.plugins.has_test() {
145-
for jest_node in iter_possible_jest_call_node(semantic) {
163+
for node in semantic.nodes() {
146164
for (rule, ctx) in &rules {
147-
rule.run_on_jest_node(&jest_node, ctx);
165+
rule.run(node, ctx);
166+
}
167+
}
168+
169+
if should_run_on_jest_node {
170+
for jest_node in iter_possible_jest_call_node(semantic) {
171+
for (rule, ctx) in &rules {
172+
rule.run_on_jest_node(&jest_node, ctx);
173+
}
174+
}
175+
}
176+
} else {
177+
for (rule, ref ctx) in rules {
178+
rule.run_once(ctx);
179+
180+
for symbol in semantic.symbols().symbol_ids() {
181+
rule.run_on_symbol(symbol, ctx);
182+
}
183+
184+
for node in semantic.nodes() {
185+
rule.run(node, ctx);
186+
}
187+
188+
if should_run_on_jest_node {
189+
for jest_node in iter_possible_jest_call_node(semantic) {
190+
rule.run_on_jest_node(&jest_node, ctx);
191+
}
148192
}
149193
}
150194
}

0 commit comments

Comments
 (0)
Please sign in to comment.