perf: don't allocate when pushing or retrieving globals #7569

arendjr · 2025-09-23T08:32:17Z

Summary

Micro-optimisation: This PR assures we don't need to do unnecessary allocations for collecting vectors when either pushing or retrieving globals.

Test Plan

Everything should stay green.

Docs

N/A

changeset-bot · 2025-09-23T08:32:21Z

⚠️ No Changeset found

Latest commit: d5c9745

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderabbitai · 2025-09-23T08:39:03Z

Walkthrough

Project-wide migration of “globals” representation from borrowed slices of &str to slices of Box. RuleContext now stores and accepts &'a [Box] and its is_global iterates and compares via as_ref(). AnalyzerOptions now returns &[Box] from globals() and accepts any iterator of Box in push_globals(). Call sites in registry and signals pass globals directly to RuleContext::new. CLI and service layers adjust collection/extension of globals, simplifying iterators and conversions without changing control flow or public signatures (except the AnalyzerOptions change).

Suggested labels

A-Linter, L-JavaScript

Suggested reviewers

dyc3
ematipico

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "perf: don't allocate when pushing or retrieving globals" accurately and concisely describes the primary change: micro‑optimisations to avoid allocations when handling globals (removing unnecessary collects/boxing and adjusting types) across multiple files. It is specific, on-topic, and informative for repository history.
Description Check	✅ Passed	The description succinctly states the intent (micro‑optimisation to avoid unnecessary allocations when collecting globals) and the test expectation, which matches the changes in the diff; it is on‑topic and sufficient for this lenient check.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b61631 and d5c9745.

📒 Files selected for processing (7)

crates/biome_analyze/src/context.rs (3 hunks)
crates/biome_analyze/src/options.rs (1 hunks)
crates/biome_analyze/src/registry.rs (1 hunks)
crates/biome_analyze/src/signals.rs (3 hunks)
crates/biome_cli/src/execute/migrate/eslint_to_biome.rs (1 hunks)
crates/biome_service/src/file_handlers/javascript.rs (1 hunks)
crates/biome_service/src/file_handlers/mod.rs (3 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

crates/biome_*/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place core crates under /crates/biome_*/

Files:

crates/biome_service/src/file_handlers/javascript.rs
crates/biome_analyze/src/context.rs
crates/biome_analyze/src/options.rs
crates/biome_analyze/src/signals.rs
crates/biome_service/src/file_handlers/mod.rs
crates/biome_cli/src/execute/migrate/eslint_to_biome.rs
crates/biome_analyze/src/registry.rs

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Format all Rust source files before committing (just f)

Files:

crates/biome_service/src/file_handlers/javascript.rs
crates/biome_analyze/src/context.rs
crates/biome_analyze/src/options.rs
crates/biome_analyze/src/signals.rs
crates/biome_service/src/file_handlers/mod.rs
crates/biome_cli/src/execute/migrate/eslint_to_biome.rs
crates/biome_analyze/src/registry.rs

🧠 Learnings (9)

📚 Learning: 2025-09-10T08:05:22.867Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings

Applied to files:

crates/biome_service/src/file_handlers/javascript.rs
crates/biome_analyze/src/signals.rs
crates/biome_service/src/file_handlers/mod.rs
crates/biome_cli/src/execute/migrate/eslint_to_biome.rs

📚 Learning: 2025-08-11T11:50:12.090Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/**/*.rs : Represent links between types using TypeReference (not Arc) to avoid cross-module retention and recursive structures; store type data in linear vectors

Applied to files:

crates/biome_service/src/file_handlers/javascript.rs
crates/biome_service/src/file_handlers/mod.rs
crates/biome_cli/src/execute/migrate/eslint_to_biome.rs

📚 Learning: 2025-09-10T08:05:22.867Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options types in `biome_rule_options` crate (one file per rule, e.g., `lib/use_my_rule.rs`)

Applied to files:

crates/biome_analyze/src/context.rs
crates/biome_analyze/src/signals.rs
crates/biome_analyze/src/registry.rs

📚 Learning: 2025-09-10T08:05:22.867Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : When emitting multiple signals, prefer `Box<[_]>` over `Vec<_>` for lower memory usage

Applied to files:

crates/biome_analyze/src/signals.rs

📚 Learning: 2025-08-11T11:50:12.090Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/src/**/*.rs : Use TypeData::Unknown for unimplemented inference and TypeData::UnknownKeyword for the explicit TypeScript unknown keyword; treat them semantically the same but keep them distinct for measurement

Applied to files:

crates/biome_cli/src/execute/migrate/eslint_to_biome.rs

📚 Learning: 2025-09-10T08:05:22.867Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : Set the `language` field in `declare_lint_rule!` to the primary language (`js`, `jsx`, `ts`, or `tsx`) the rule targets

Applied to files:

crates/biome_cli/src/execute/migrate/eslint_to_biome.rs

📚 Learning: 2025-08-11T11:46:05.836Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper advice types from biome_diagnostics::v2 (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) when suitable

Applied to files:

crates/biome_cli/src/execute/migrate/eslint_to_biome.rs

📚 Learning: 2025-08-11T11:50:12.090Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/biome_module_graph/src/js_module_info/collector.rs : Thin (module-level) inference may resolve qualifiers to local declarations or globals (TypeReference::Resolved), mark imported bindings as TypeReference::Import, and set unresolved to TypeReference::Unknown; it must not look beyond the current module

Applied to files:

crates/biome_cli/src/execute/migrate/eslint_to_biome.rs

📚 Learning: 2025-09-10T08:05:22.867Z

Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/**/nursery/**/*.rs : Place all new rules in the nursery group (implement rule files under a `.../src/*/nursery/` directory)

Applied to files:

crates/biome_analyze/src/registry.rs

🧬 Code graph analysis (4)

crates/biome_analyze/src/context.rs (2)

crates/biome_analyze/src/options.rs (1)

globals (155-157)

crates/biome_analyze/src/rule.rs (1)

globals (508-530)

crates/biome_analyze/src/signals.rs (1)

crates/biome_analyze/src/options.rs (1)

globals (155-157)

crates/biome_cli/src/execute/migrate/eslint_to_biome.rs (1)

crates/biome_analyze/src/options.rs (1)

globals (155-157)

crates/biome_analyze/src/registry.rs (1)

crates/biome_analyze/src/options.rs (1)

globals (155-157)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)

GitHub Check: Bench (biome_configuration)
GitHub Check: Bench (biome_module_graph)
GitHub Check: Bench (biome_json_analyze)
GitHub Check: Bench (biome_package)
GitHub Check: Bench (biome_html_parser)
GitHub Check: Bench (biome_html_formatter)
GitHub Check: Bench (biome_graphql_parser)
GitHub Check: Bench (biome_json_formatter)
GitHub Check: Bench (biome_js_parser)
GitHub Check: Bench (biome_graphql_formatter)
GitHub Check: Bench (biome_json_parser)
GitHub Check: Bench (biome_js_formatter)
GitHub Check: Bench (biome_css_analyze)
GitHub Check: Bench (biome_css_parser)
GitHub Check: Bench (biome_js_analyze)
GitHub Check: Bench (biome_css_formatter)
GitHub Check: Test Node.js API
GitHub Check: Test (depot-ubuntu-24.04-arm-16)
GitHub Check: Check Dependencies
GitHub Check: autofix
GitHub Check: Test (depot-windows-2022-16)
GitHub Check: Documentation
GitHub Check: Lint project (depot-windows-2022)
GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)

🔇 Additional comments (13)

crates/biome_cli/src/execute/migrate/eslint_to_biome.rs (1)

345-345: LGTM! Type inference simplification aligns with the PR's memory optimisation goals.

The removal of the explicit FxHashSet<_> type annotation lets Rust infer the collection type, which is consistent with the PR's objective to reduce allocations when handling globals.

crates/biome_service/src/file_handlers/javascript.rs (1)

310-310: Good optimisation - direct extension without intermediate collection.

This change eliminates the unnecessary intermediate Vec allocation by directly extending with the iterator returned from override_js_globals. This aligns perfectly with the PR's goal of avoiding allocations when handling globals.

crates/biome_service/src/file_handlers/mod.rs (3)

1034-1034: Clever use of copied() and Into::into() to avoid boxing.

The change from Box::from(*s) to copied().map(Into::into) avoids explicit boxing while maintaining the same semantics. This pattern efficiently converts &str to Box<str> without intermediate allocations.

1088-1089: Consistent application of the allocation-free pattern.

Good to see the same optimisation applied here as well. The consistency across the codebase makes the changes easier to understand and maintain.

1099-1099: Pattern consistently applied across all global handling code paths.

All three instances of global collection now use the same efficient pattern, maintaining code consistency whilst achieving the PR's optimisation goals.

crates/biome_analyze/src/registry.rs (1)

415-415: Correct adaptation to the new by-value globals parameter.

The change from &globals to globals aligns with the updated RuleContext::new signature that now expects globals by value. This is consistent with the broader refactoring to use &'a [Box<str>] throughout the codebase.

crates/biome_analyze/src/signals.rs (3)

366-366: By-value passing consistently applied across all signal contexts.

All three call sites (diagnostic, actions, transformations) now pass globals by value to RuleContext::new, maintaining consistency with the updated API. The change preserves functionality whilst aligning with the new ownership model.

403-403: Actions context properly updated.

468-468: Transformations context properly updated.

crates/biome_analyze/src/options.rs (1)

151-157: Well-designed API changes for flexible global handling.

The updated signatures provide good flexibility:

push_globals now accepts any IntoIterator<Item = Box<str>>, making it more ergonomic for callers

globals() returns &[Box<str>] directly, exposing the internal storage efficiently without conversion

These changes support the PR's goal of reducing allocations whilst maintaining a clean API.

crates/biome_analyze/src/context.rs (3)

35-35: Constructor signature aligns with the field; LGTM.

158-161: Allocation‑free membership check reads clearly; LGTM.

as_ref() + any keeps it lean. If this ever becomes a hot path with many globals, we can revisit a prebuilt set—out of scope for this perf tweak.

16-16: Switching to &[Box] — callers already pass slices; no per‑call boxing found

AnalyzerOptions keeps globals as Vec<Box> and exposes them via globals(&self) -> &[Box]; signals.rs and registry.rs call options.globals() and pass that slice directly into RuleContext::new. file_handlers/mod.rs uses push_globals at configuration time to extend AnalyzerOptions, so any Box allocation happens once at config time, not per invocation.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codspeed-hq · 2025-09-23T08:45:45Z

CodSpeed Performance Report

Merging #7569 will not alter performance

_{Comparing arendjr:no-alloc-globals (d5c9745) with main (1b61631)}

Summary

✅ 133 untouched

perf: don't allocate when pushing or retrieving globals

d5c9745

github-actions bot added A-CLI Area: CLI A-Project Area: project A-Linter Area: linter labels Sep 23, 2025

arendjr merged commit b8d2a71 into biomejs:main Sep 23, 2025
29 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

perf: don't allocate when pushing or retrieving globals #7569

perf: don't allocate when pushing or retrieving globals #7569

Uh oh!

arendjr commented Sep 23, 2025

Uh oh!

changeset-bot bot commented Sep 23, 2025

Uh oh!

coderabbitai bot commented Sep 23, 2025

Uh oh!

codspeed-hq bot commented Sep 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

perf: don't allocate when pushing or retrieving globals #7569

perf: don't allocate when pushing or retrieving globals #7569

Uh oh!

Conversation

arendjr commented Sep 23, 2025

Summary

Test Plan

Docs

Uh oh!

changeset-bot bot commented Sep 23, 2025

⚠️ No Changeset found

Uh oh!

coderabbitai bot commented Sep 23, 2025

Walkthrough

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

codspeed-hq bot commented Sep 23, 2025

CodSpeed Performance Report

Merging #7569 will not alter performance

Summary

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant