perf: minor performance tweaks #7122

siketyan · 2025-08-05T18:27:02Z

Summary

Performed some minor performance tweaks.
In my private repo, this reduced ~700ms of the execution time.

Test Plan

N/A

Docs

N/A

changeset-bot · 2025-08-05T18:27:06Z

⚠️ No Changeset found

Latest commit: 17ca35e

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-08-05T18:27:09Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches

🧪 Generate unit tests

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

siketyan · 2025-08-05T18:28:02Z

crates/biome_fs/src/path.rs

-        let kind = if path.is_dir() {
-            FileKinds::Directory
-        } else {
-            path.file_name().map(Self::priority).unwrap_or_default()
-        };


This is introduced in 4309234, but it seems not used. Utf8Path::is_dir() requires an I/O syscall, that is relatively heavy when it's called many times.

I've been bothered by that one for a while, and wished to remove it too. Glad you did!

siketyan · 2025-08-05T18:28:39Z

crates/biome_service/src/settings.rs

-            let ignore_directory = if gitignore.path().is_file() {
-                // SAFETY: if it's a file, it always has a parent
-                gitignore.path().parent().unwrap()
-            } else {
-                gitignore.path()
-            };


Based on doc comments, gitignore.path() is always a directory path where contains the .gitignore file.

I hope you're right 😅 This may be something @ematipico can say better than I can.

It's a good change. I initially added this because I didn't understand completely how the library works. But now we always store folders, so this piece of code can go away ☺️

siketyan · 2025-08-05T18:30:28Z

crates/biome_service/src/workspace/server.rs

-                        Vec::new()
-                    },
-                });
+        let (diagnostics, errors, skipped_diagnostics) = if categories.is_lint()


format_with_guard() calls Workspace::pull_diagnostics to collect parse diagnostics, but it also lead calling lint(), which is not necessary and takes some time.

siketyan · 2025-08-05T18:35:18Z

crates/biome_cli/tests/snapshots/main_commands_check/suppression_syntax_error.snap

-  i Example of suppression: // biome-ignore lint: reason
-  
-
 ```


This was duplicated diagnostic!

Nice catch!

codspeed-hq · 2025-08-05T18:40:22Z

CodSpeed Performance Report

Merging #7122 will not alter performance

_{Comparing siketyan:perf/minor-tweaks (17ca35e) with next (85b1128)¹}

Summary

✅ 128 untouched benchmarks

No successful run was found on next (76df523) during the generation of this report, so 85b1128 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

arendjr

LGTM, but I would recommend to check the gitignore thing with Ema.

arendjr · 2025-08-06T07:14:05Z

crates/biome_cli/tests/snapshots/main_commands_check/suppression_syntax_error.snap

-  i Example of suppression: // biome-ignore lint: reason
-  
-
 ```


Nice catch!

arendjr · 2025-08-06T07:14:38Z

crates/biome_fs/src/path.rs

-        let kind = if path.is_dir() {
-            FileKinds::Directory
-        } else {
-            path.file_name().map(Self::priority).unwrap_or_default()
-        };


I've been bothered by that one for a while, and wished to remove it too. Glad you did!

arendjr · 2025-08-06T07:15:50Z

crates/biome_service/src/settings.rs

-            let ignore_directory = if gitignore.path().is_file() {
-                // SAFETY: if it's a file, it always has a parent
-                gitignore.path().parent().unwrap()
-            } else {
-                gitignore.path()
-            };


I hope you're right 😅 This may be something @ematipico can say better than I can.

arendjr · 2025-08-06T07:19:02Z

crates/biome_service/src/workspace/server.rs

+        let (diagnostics, errors, skipped_diagnostics) = if (categories.is_lint()
+            || categories.is_assist())


When do we call pull_diagnostics() when the category is not either lint or assist? I suppose this change is what fixed the duplicate diagnostic, so it seems a good change, but I'm curious at why this works...

@arendjr format_with_guard() calls pull_diagnostics() with RuleCategories::Syntax.

biome/crates/biome_cli/src/execute/process_file/format.rs

Lines 37 to 45 in b9642ab

let diagnostics_result = workspace_file

.guard()

.pull_diagnostics(

RuleCategoriesBuilder::default().with_syntax().build(),

Vec::new(),

Vec::new(),

false, // NOTE: probably to revisit

)

.with_file_path_and_code(workspace_file.path.to_string(), category!("format"))?;

Can you add a test that tries to format a code that violates a syntax rule? Ideally it shouldn't format it, and it should emit the diagnostic

Added here: 17ca35e

siketyan self-assigned this Aug 5, 2025

github-actions bot added A-CLI Area: CLI A-Core Area: core A-Project Area: project A-Linter Area: linter labels Aug 5, 2025

siketyan commented Aug 5, 2025

View reviewed changes

perf: minor performance tweaks

420fdcf

siketyan force-pushed the perf/minor-tweaks branch from 431fbd6 to 420fdcf Compare August 5, 2025 18:35

siketyan commented Aug 5, 2025

View reviewed changes

siketyan requested review from a team August 5, 2025 18:38

siketyan marked this pull request as ready for review August 5, 2025 18:38

arendjr approved these changes Aug 6, 2025

View reviewed changes

siketyan requested a review from ematipico August 6, 2025 07:26

test: biome format should not write files with syntax errors

17ca35e

ematipico approved these changes Aug 6, 2025

View reviewed changes

siketyan merged commit d4e0e64 into biomejs:next Aug 6, 2025
28 checks passed

ematipico pushed a commit that referenced this pull request Aug 12, 2025

perf: minor performance tweaks (#7122)

66cd612

		let (diagnostics, errors, skipped_diagnostics) = if (categories.is_lint()
		\|\| categories.is_assist())

	let diagnostics_result = workspace_file
	.guard()
	.pull_diagnostics(
	RuleCategoriesBuilder::default().with_syntax().build(),
	Vec::new(),
	Vec::new(),
	false, // NOTE: probably to revisit
	)
	.with_file_path_and_code(workspace_file.path.to_string(), category!("format"))?;

Uh oh!

perf: minor performance tweaks #7122

perf: minor performance tweaks #7122

Uh oh!

Conversation

siketyan commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Docs

Uh oh!

changeset-bot bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

coderabbitai bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #7122 will not alter performance

Summary

Footnotes

Uh oh!

arendjr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

siketyan commented Aug 5, 2025 •

edited

Loading

changeset-bot bot commented Aug 5, 2025 •

edited

Loading

coderabbitai bot commented Aug 5, 2025 •

edited

Loading

codspeed-hq bot commented Aug 5, 2025 •

edited

Loading