Skip to content

Conversation

@camchenry
Copy link
Member

@camchenry camchenry commented Mar 28, 2025

Previously, we were wasting some time creating a Vec at runtime with the lintable extensions, which is all of the JS/TS extensions plus other frameworks extensions like .vue. Now, we precompute this as a static string slice at compile time. This also ensures that we are using the same extensions list between oxlint and the language server too. Theoretically this is a very minor perf win possibly?

Copy link
Member Author

camchenry commented Mar 28, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-editor Area - Editor and Language Server C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Mar 28, 2025
@camchenry camchenry marked this pull request as ready for review March 28, 2025 04:22
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 28, 2025

CodSpeed Instrumentation Performance Report

Merging #10090 will not alter performance

Comparing 03-28-refactor_linter_compute_lintable_extensions_at_compile_time (d8e49a1) with main (3d2888e)

Summary

✅ 33 untouched benchmarks

@graphite-app graphite-app bot changed the base branch from 03-27-fix_linter_enable_multi-file_analysis_for_nested_configs to graphite-base/10090 March 28, 2025 05:29
@graphite-app graphite-app bot force-pushed the graphite-base/10090 branch from 4d973b9 to 2c80858 Compare March 28, 2025 06:50
@graphite-app graphite-app bot force-pushed the 03-28-refactor_linter_compute_lintable_extensions_at_compile_time branch from 612a20a to 3ca18db Compare March 28, 2025 06:50
@graphite-app graphite-app bot changed the base branch from graphite-base/10090 to main March 28, 2025 06:51
@graphite-app graphite-app bot force-pushed the 03-28-refactor_linter_compute_lintable_extensions_at_compile_time branch from 3ca18db to 014adf6 Compare March 28, 2025 06:51
Copy link
Member

@Sysix Sysix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Don't know if constcat is really needed or not

@camchenry
Copy link
Member Author

LGTM. Don't know if constcat is really needed or not

Would love to know if there's a solution in the standard library for concatening const string slices, since I couldn't find one. Seemed like it more or less required defining a custom macro. As far as dependencies go, it seems like constcat is quite small (only ~8KB).

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Mar 29, 2025
Copy link
Member

Boshen commented Mar 29, 2025

Merge activity

  • Mar 29, 8:22 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Mar 29, 8:22 AM EDT: Boshen added this pull request to the Graphite merge queue.
  • Mar 29, 8:47 AM EDT: A user merged this pull request with the Graphite merge queue.

graphite-app bot pushed a commit that referenced this pull request Mar 29, 2025
Previously, we were wasting some time creating a `Vec` at runtime with the lintable extensions, which is all of the JS/TS extensions plus other frameworks extensions like `.vue`. Now, we precompute this as a static string slice at compile time. This also ensures that we are using the same extensions list between `oxlint` and the language server too. Theoretically this is a very minor perf win possibly?
@graphite-app graphite-app bot force-pushed the 03-28-refactor_linter_compute_lintable_extensions_at_compile_time branch from 014adf6 to c9dbbf8 Compare March 29, 2025 12:35
Previously, we were wasting some time creating a `Vec` at runtime with the lintable extensions, which is all of the JS/TS extensions plus other frameworks extensions like `.vue`. Now, we precompute this as a static string slice at compile time. This also ensures that we are using the same extensions list between `oxlint` and the language server too. Theoretically this is a very minor perf win possibly?
@graphite-app graphite-app bot force-pushed the 03-28-refactor_linter_compute_lintable_extensions_at_compile_time branch from ea0830b to d8e49a1 Compare March 29, 2025 12:38
@graphite-app graphite-app bot merged commit d8e49a1 into main Mar 29, 2025
31 checks passed
@graphite-app graphite-app bot deleted the 03-28-refactor_linter_compute_lintable_extensions_at_compile_time branch March 29, 2025 12:47
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Mar 29, 2025
Boshen added a commit that referenced this pull request Apr 1, 2025
## [0.16.4] - 2025-04-01

- da6336c language_server: [**BREAKING**] Remove `enable` configuration,
the client should shutdown the server instead (#9990) (Sysix)

### Features

- 06e3db9 linter: Support `multipleFileExtensions` option for
`unicorn/filename-case` (#10118) (shulaoda)
- dbe0e46 linter: Support `ignore` option for `unicorn/filename-case`
(#10107) (shulaoda)

### Bug Fixes

- 0a33e27 editor: Update `initializationOptions` for a possible restart
(#10121) (Sysix)
- ac780a2 editor: Repair filewatchers when no custom config provided
(#10104) (Sysix)
- 4303ace editor: Dont send `didChangeConfiguration` request to the
server when it is shutdown (#10084) (Sysix)
- 07f2a25 editor: Enable regex parsing in language server (#10035)
(camchenry)
- fcf7702 language_server: Start from a default oxlint configuration +
SafeFix for nested configuration (#10043) (Sysix)
- aba3654 linter: Span disable directive correctly on next line (#10141)
(Ulrich Stark 🦀)
- 2c80858 linter: Enable multi-file analysis for nested configs (#10089)
(camchenry)

### Performance

- 566be59 linter: Replace `phf_set` with `array` in `eslint/func-names`
(#10119) (shulaoda)
- 5e14fe9 linter: Inline `PRE_DEFINE_VAR` and use `array` format
(#10079) (shulaoda)

### Refactor

- 327be53 editor: `LanguageClient` can be undefined (#10112) (Sysix)
- 5ec477c editor: Make `onConfigChange` async (#10110) (Sysix)
- a278d73 editor: Use warning background when the plugin is deactived
(#10085) (Sysix)
- c0e5251 language_server: Set `null` as a default value for
`configPath` (#10047) (Sysix)
- 09c0ac6 linter: Improve `unicorn/filename-case` (#10117) (shulaoda)
- d8e49a1 linter: Compute lintable extensions at compile time (#10090)
(camchenry)
- b3ec235 linter: Use items of `oxc_ast::ast` module directly (#10100)
(Ulrich Stark 🦀)
- 93e6c0b linter: Use `FormalParameter::has_modifier` to detect
parameter properties (#10097) (Ulrich Stark 🦀)
- 5d829c2 semantic: Align handling of declaring symbol for function with
TypeScript (#10086) (Dunqing)

### Testing

- 410b8d6 editor: Use debug build of the language server (#10083)
(Sysix)
- 500add0 editor: Add test for `oxc.fixAll` command (#10045) (Sysix)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-editor Area - Editor and Language Server A-linter Area - Linter C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants