Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 slow linter on apply #1569

Closed
1 task done
nyanrus opened this issue Jan 16, 2024 · 8 comments
Closed
1 task done

🐛 slow linter on apply #1569

nyanrus opened this issue Jan 16, 2024 · 8 comments

Comments

@nyanrus
Copy link

nyanrus commented Jan 16, 2024

Environment information

CLI:
  Version:                      1.5.2
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           windows

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v20.9.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "pnpm/8.14.1"

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 true

Workspace:
  Open Documents:               0

What happened?

  1. clone https://github.com/nyanrus/Floorp-core (originally cloned from Floorp-Projects/Floorp-core)
  2. run
pnpm add --save-dev --save-exact @biomejs/biome
pnpm dlx @biomejs/biome init
pnpm biome lint ./browser --apply
  1. linter is fast, but applying is too slow (1891ms vs 147s)

Expected result

the applying of the linter should be faster

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@nyanrus
Copy link
Author

nyanrus commented Jan 16, 2024

may related to #98

@nyanrus nyanrus changed the title 🐛 biome linter is too slow in not linted big project 🐛 biome linter applying is too slow in not linted big project Jan 16, 2024
@nyanrus nyanrus changed the title 🐛 biome linter applying is too slow in not linted big project 🐛 slow linter on apply Jan 16, 2024
@believer
Copy link

believer commented Jan 16, 2024

We're also seeing some speed issues with 1.5.2 when running biome check --apply .. On 1.4.1 the command takes ~140 ms, but on 1.5.2 it takes ~1800 ms. Same amount of files, 497, listed as checked.

Also, biome-vscode seems to still be running 1.4.1, so there are some mismatches between the CLI and the results in VS Code.

@nyanrus
Copy link
Author

nyanrus commented Jan 16, 2024

It seems linter filesystem problem because the format --write is fast enough
and linter without --apply is fast too.

@ematipico
Copy link
Member

We're also seeing some speed issues with 1.5.2 when running biome check --apply .. On 1.4.1 the command takes ~140 ms, but on 1.5.2 it takes ~1800 ms. Same amount of files, 497, listed as checked.

Are you able to give us more information using debug logging? --log-level=debug

Also, biome-vscode seems to still be running 1.4.1, so there are some mismatches between the CLI and the results in VS Code.

Try to restart the LSP server; that's possibly the reason.

@ematipico
Copy link
Member

It seems linter filesystem problem because the format --write is fast enough and linter without --apply is fast too.

The issue is due to a performance bottleneck when we apply the code actions to big files. We haven't been able to allocate enough time to understand and fix this, unfortunately.

@believer
Copy link

Are you able to give us more information using debug logging? --log-level=debug

The most notable thing I can see from the debug logging is that there's a bunch of Unknown RomePath logs, even from folders that we have set in files.ignore:

 2024-01-16T11:31:42.720218Z DEBUG  File capabilities: Unknown RomePath { path: "./dist/apps/api/src/lib/index.js.map" }
    at crates/biome_service/src/workspace/server.rs:90 on biome::worker_1

Our biome.json looks something like this:

{
  "$schema": "https://biomejs.dev/schemas/1.5.2/schema.json",
  "files": {
    "ignore": [
      ".vscode/**",
      "android/**",
      "ios/**",
      "dist/**"
    ],
    "ignoreUnknown": true
  },
  "formatter": {
    "indentStyle": "space"
  },
  "linter": {
    "enabled": true,
    "rules": {
      "recommended": true
    }
  },
  "organizeImports": {
    "enabled": true
  },
  "javascript": {
    "formatter": {
      "quoteStyle": "single",
      "semicolons": "asNeeded",
      "arrowParentheses": "always",
      "trailingComma": "es5"
    }
  },
  "overrides": [
    {
      "include": ["apps/app/**"],
      "linter": {
        "rules": {
          "suspicious": {
            "noConsoleLog": "error"
          },
          "style": {
            "useBlockStatements": "error"
          }
        }
      }
    },
    {
      "include": [
        "apps/api/**",
        "libs/data-aggregator/**"
      ],
      "linter": {
        "rules": {
          "suspicious": {
            "noConsoleLog": "off"
          },
          "complexity": {
            "noForEach": "off"
          }
        }
      }
    }
  ]
}

If we remove the overrides, biome check --apply . runs in about half the time listed above.

Try to restart the LSP server; that's possibly the reason.

Ahh, of course it should use the local version first! This PR to update confused us.

@si14
Copy link
Contributor

si14 commented Feb 6, 2024

Seeing a potentially related problem on 1.5.3 with a large file in vscode. Direct reformatting requests are instant, but saves with import formatting take a few seconds.

Test editor configured like this:

    "editor.codeActionsOnSave": {
        "source.organizeImports.biome": "always"
    },

Biome configured like this

Disabling import formatting with

	"organizeImports": {
		"enabled": false
	},

…makes saves instant.

I tried enabling logs for the extension, here's the relevant bit that made me think it's related to #1569 (comment) :

[Trace - 17:11:44] Sending notification 'textDocument/didChange'.
[Trace - 17:11:44] Received notification 'textDocument/publishDiagnostics'.
[Trace - 17:11:45] Sending request 'textDocument/codeAction - (5)'.
[Trace - 17:11:48] Received response 'textDocument/codeAction - (5)' in 3542ms.
[Trace - 17:11:48] Sending request 'textDocument/formatting - (6)'.
[Trace - 17:11:48] Received response 'textDocument/formatting - (6)' in 24ms.
[Trace - 17:11:48] Sending notification 'textDocument/didChange'.
[Trace - 17:11:48] Received notification 'textDocument/publishDiagnostics'.
[Trace - 17:11:48] Sending notification 'textDocument/didSave'.

@si14 si14 mentioned this issue Feb 7, 2024
1 task
github-merge-queue bot pushed a commit to tldraw/tldraw that referenced this issue Feb 7, 2024
Biome as it is now didn't work out for us 😢 

Summary for posterity:

* it IS much, much faster, fast enough to skip any sort of caching
* we couldn't fully replace Prettier just yet. We use Prettier
programmatically to format code in docs, and Biome's JS interface is
officially alpha and [had legacy peer deps
set](biomejs/biome#1756) (which would fail our
CI build as we don't allow installation warnings)
* ternary formatting differs from Prettier, leading to a large diff
biomejs/biome#1661
* import sorting differs from Prettier's
`prettier-plugin-organize-imports`, making the diff even bigger
* the deal breaker is a multi-second delay on saving large files (for us
it's
[Editor.ts](https://github.com/tldraw/tldraw/blob/main/packages/editor/src/lib/editor/Editor.ts))
in VSCode when import sorting is enabled. There is a seemingly relevant
Biome issue where I posted a small summary of our findings:
biomejs/biome#1569 (comment)

Further actions:

* reevaluate in a few months as Biome matures

### Change Type

- [x] `internal` — Any other changes that don't affect the published
package
@believer
Copy link

This issue has been resolved for us in 1.6.0. Might be due to the new behavior of "Don't process files under an ignored directory."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants