🐛 rome is insatiable when it comes to CPU resources on windows

GeorgeTailor commented 1 year ago

Environment information

windows 11, visual studio code.
I also have visual studio code rome extension installed, it might be causing the problem and not the rome itself.

What happened?

on a rather small JS file (around 2 KB) it can randomly hang and start eating CPU time, probably there's an infinite loop somewhere or something similar. This also happens on any other file extensions.

Expected result

rome should not be consuming 5% of a 12th Gen Intel(R) Core(TM) i9-12900K 3.20 GHz processor to format a 2 KB javascript file forever/until it is killed/file is closed.

Code of Conduct

[X] I agree to follow Rome's Code of Conduct

ematipico commented 1 year ago

It would be useful to have more information, for example:

the version of the extension
what kind of operations you're doing
a code that allows to reproduce the issue
possibly, some logs. Can find the logs in the temporary folder of your file system, under rome-logs

GeorgeTailor commented 1 year ago

version — 0.22.0, I can see it was release on Mar 28, but this happened in previous version as well.
operation — formatting the document with shortcut alt+shift+F
code is irrelevant, it happens randomly, but I suspect only JS formatting is causing this, typescript seems to be fine, I'll update this message if I find anything else.
unfortunately, I cannot seem to find anything, my AppData does not have anything related to rome, neither in Roaming nor Local nor LocalLow, where does rome store its logs?

GeorgeTailor commented 1 year ago

so, this happened again on 1 KB typescript file, however the rome logs are empty

When I tried to revert formatting and format again the issue did not happen

ematipico commented 1 year ago

I don't have a Windows OS, so I can't tell where its temporary folder is. I found this on Google: https://answers.microsoft.com/en-us/windows/forum/all/where-is-the-temporary-folder/44a039a5-45ba-48dd-84db-fd700e54fd56

Here's a brief explanation of where to find the logs: https://rustdocs.rome.tools/rome_cli/index.html#logs

GeorgeTailor commented 1 year ago

ok, those files are quite bulky. One off thing I've noticed is that it opens files in node_modules, for some reason:

rome_lsp::handlers::text_document::did_open{params=DidOpenTextDocumentParams { text_document: TextDocumentItem { uri: Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "%path%/node_modules/playwright-core/types/types.d.ts", query: None, fragment: None }

is it supposed to do that? Or do I have to explicitly tell rome to ignore node_modules folder?

apart from that I can see that rome fails to read rome.json properly, at least that's what the logs say, but it reads it anyway???:

├─┐rome_lsp::server::initialized{params=InitializedParams}
│ ├─0ms INFO rome_lsp::server Attempting to load the configuration from 'rome.json' file
│ ├─┐rome_lsp::session::load_extension_settings{}
│ ├─┐rome_lsp::session::load_workspace_settings{}
│ │ ├─0ms INFO rome_service::configuration Attempting to read the configuration file from path_to_project\rome.json
│ │ ├─┐rome_fs::fs::os::OsFileSystem::open_with_options{path="path_to_project\\rome.json", options=OpenOptions { read: true, write: false, truncate: false, create: false, create_new: false }}
│ │ ├─┘
│ │ ├─┐rome_fs::fs::os::OsFile::read_to_string{}
│ │ ├─┘
│ │ ├─┐rome_json_parser::parse{}
│ │ ├─┘
│ │ ├─0ms WARN rome_lsp::session The deserialization of the configuration resulted in errors. Rome will its defaults where possible.
│ │ ├─0ms INFO rome_lsp::session Loaded workspace settings: Configuration {
│ │ │     schema: Some(
│ │ │         "\"./node_modules/rome/configuration_schema.json\"",
│ │ │     ),
│ │ │     files: Some(
│ │ │         FilesConfiguration {
│ │ │             max_size: None,
│ │ │             ignore: Some(
│ │ │                 {
│ │ │                     "coverage/**",
│ │ │                     "public/js/rich-text-editor.js",
│ │ │                     "*.json",
│ │ │                 },
│ │ │             ),
│ │ │         },
│ │ │     ),
│ │ │     formatter: Some(
│ │ │         FormatterConfiguration {
│ │ │             enabled: true,
│ │ │             format_with_errors: false,
│ │ │             indent_style: Tab,
│ │ │             indent_size: 2,
│ │ │             line_width: LineWidth(
│ │ │                 140,
│ │ │             ),
│ │ │             ignore: None,
│ │ │         },
│ │ │     ),
│ │ │     organize_imports: Some(
│ │ │         OrganizeImports {
│ │ │             enabled: false,
│ │ │             ignore: None,
│ │ │         },
│ │ │     ),
│ │ │     linter: Some(
│ │ │         LinterConfiguration {
│ │ │             enabled: true,
│ │ │             rules: Some(
│ │ │                 Rules {
│ │ │                     recommended: Some(
│ │ │                         true,
│ │ │                     ),
│ │ │                     all: None,
│ │ │                     a11y: None,
│ │ │                     complexity: None,
│ │ │                     correctness: None,
│ │ │                     nursery: None,
│ │ │                     performance: None,
│ │ │                     security: None,
│ │ │                     style: None,
│ │ │                     suspicious: None,
│ │ │                 },
│ │ │             ),
│ │ │             ignore: None,
│ │ │         },
│ │ │     ),
│ │ │     javascript: None,
│ │ │ }
│ │ ├─┐rome_service::workspace::server::update_settings{params=UpdateSettingsParams { configuration: Configuration { schema: Some("\"./node_modules/rome/configuration_schema.json\""), files: Some(FilesConfiguration { max_size: None, ignore: Some({"coverage/**", "public/js/rich-text-editor.js", "*.json"}) }), formatter: Some(FormatterConfiguration { enabled: true, format_with_errors: false, indent_style: Tab, indent_size: 2, line_width: LineWidth(140), ignore: None }), organize_imports: Some(OrganizeImports { enabled: false, ignore: None }), linter: Some(LinterConfiguration { enabled: true, rules: Some(Rules { recommended: Some(true), all: None, a11y: None, complexity: None, correctness: None, nursery: None, performance: None, security: None, style: None, suspicious: None }), ignore: None }), javascript: None } }}
│ │ │ ├─┐rome_service::settings::merge_with_configuration{configuration=Configuration { schema: Some("\"./node_modules/rome/configuration_schema.json\""), files: Some(FilesConfiguration { max_size: None, ignore: Some({"coverage/**", "public/js/rich-text-editor.js", "*.json"}) }), formatter: Some(FormatterConfiguration { enabled: true, format_with_errors: false, indent_style: Tab, indent_size: 2, line_width: LineWidth(140), ignore: None }), organize_imports: Some(OrganizeImports { enabled: false, ignore: None }), linter: Some(LinterConfiguration { enabled: true, rules: Some(Rules { recommended: Some(true), all: None, a11y: None, complexity: None, correctness: None, nursery: None, performance: None, security: None, style: None, suspicious: None }), ignore: None }), javascript: None }}
│ │ │ ├─┘
│ │ ├─┘
│ ├─┘

here's my config:

{
    "$schema": "./node_modules/rome/configuration_schema.json",
    "linter": {
        "enabled": true,
        "rules": {
            "recommended": true
        }
    },
    "formatter": {
        "enabled": true,
        "indentStyle": "tab",
        "indentSize": 4,
        "lineWidth": 140
    },
    "files": {
        "ignore": ["coverage/**", ".wrangler/**", "public/js/components.js", "dist/**", "dist-local/**"]
    }
}

GeorgeTailor commented 1 year ago

I think I know how to replicate it.

Use visual studio code with rome extension;

Create a whatever.ts file with the following content:

const userNoTabsTemplate = <HTMLTemplateElement>document.getElementById("document-user-no-tabs-row-el");

ctrl + left mouse click on HTMLTemplateElement to open lib.dom.ts
go back to whatever.ts and try to format the file;
the process should hang.

I was also able to replicate it with JS files:

same as above

create file sw.js with the following content:

self.addEventListener("install", function (event) {
const offlineRequest = new Request("offline.html");
event.waitUntil(
    fetch(offlineRequest).then(function (response) {
        return caches.open("offline").then(function (cache) {
            console.log("[oninstall] Cached offline page", response.url);
            return cache.put(offlineRequest, response);
        });
    }),
);
});

ctrl + left mouse click on Request to open typescript.lib;
go back to your file and try to format it;
the process should hang.

In the logs there's a huge output of the whole ts file from node_modules and something like the following:

│ │ ├─743ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─743ms DEBUG rome_rowan::ast::batch  changes [CommitChange { parent_depth: 2, parent: Some(JS_MODULE_ITEM_LIST@0..824026), parent_range: Some((0, 824026)), new_node_slot: 855, new_node: Some(Node(TS_TYPE_ALIAS_DECLARATION@0..82)) }]
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─784ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─785ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─785ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─785ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─785ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─785ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─785ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─785ms DEBUG rome_rowan::ast::batch pushing change...
...
[Whitespace(" ")])) }]
│ │ ├─35272ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35272ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35272ms DEBUG rome_rowan::ast::batch  changes [CommitChange { parent_depth: 5, parent: Some(JS_VARIABLE_DECLARATION@806863..806888), parent_range: Some((806863, 806888)), new_node_slot: 0, new_node: Some(Token(LET_KW@0..4 "let" [] [Whitespace(" ")])) }]
│ │ ├─35345ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35345ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35345ms DEBUG rome_rowan::ast::batch  changes [CommitChange { parent_depth: 5, parent: Some(JS_VARIABLE_DECLARATION@806898..806926), parent_range: Some((806898, 806926)), new_node_slot: 0, new_node: Some(Token(LET_KW@0..4 "let" [] [Whitespace(" ")])) }]
│ │ ├─35419ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35420ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35420ms DEBUG rome_rowan::ast::batch  changes [CommitChange { parent_depth: 5, parent: Some(JS_VARIABLE_DECLARATION@806936..806954), parent_range: Some((806936, 806954)), new_node_slot: 0, new_node: Some(Token(LET_KW@0..4 "let" [] [Whitespace(" ")])) }]
│ │ ├─35491ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35491ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35491ms DEBUG rome_rowan::ast::batch  changes [CommitChange { parent_depth: 5, parent: Some(JS_VARIABLE_DECLARATION@806964..806992), parent_range: Some((806964, 806992)), new_node_slot: 0, new_node: Some(Token(LET_KW@0..4 "let" [] [Whitespace(" ")])) }]
│ │ ├─35566ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35566ms DEBUG rome_rowan::ast::batch pushing change...
│ │ ├─35566ms DEBUG rome_rowan::ast::batch  changes [CommitChange { parent_depth: 5, parent: Some(JS_VARIABLE_DECLARATION@807975..808002), parent_range: Some((807975, 808002)), new_node_slot: 0, new_node: Some(Token(LET_KW@0..4 "let" [] [Whitespace(" ")])) }]
│ ├─┘
├─┘
├─┐rome_lsp::handlers::text_document::did_close{params=DidCloseTextDocumentParams { text_document: TextDocumentIdentifier { uri: Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "/d%3A/programming/Microsoft%20VS%20Code/resources/app/extensions/node_modules/typescript/lib/lib.dom.d.ts", query: None, fragment: None } } }}
├─┘

ematipico commented 1 year ago

apart from that I can see that rome fails to read rome.json properly, at least that's what the logs say, but it reads it anyway???:

That's a bug in the code that I just noticed. You should not see that warning :) the file is correct, and Rome didn't detect errors!

Thank you for the thorough information!

rome_lsp::handlers::text_document::did_open{params=DidOpenTextDocumentParams { text_document: TextDocumentItem { uri: Url { scheme: "file", cannot_be_a_base: false, username: "", password: None, host: None, port: None, path: "%path%/node_modules/playwright-core/types/types.d.ts", query: None, fragment: None }

The path coming from this event is weird 🤔 I will have to look at that.

From the logs and your info it seems that you also installed rome via a package manager. That wasn't clear from your report because you didn't attach the rome rage output to the issue. Could you please do so? That command is handy for us! 😄

GeorgeTailor commented 1 year ago

I replaced my actual path like D:/whatever/project-name with %path%.

Yes, I also run rome lint during build phase, and the rome itself is installed from npm. The version is 12.0.0. rome rage outputs similar stuff to what I previously posted, it does not fit into my terminal, that's like 20-40KB of text.

What I also noticed is that when I install 11.0.0 the issue is not there. So, the extension probably falls back to locally installed version, it seems, and it only happens on 12.0.0

jpike88 commented 1 year ago

Almost definitely related to https://github.com/rome/tools/issues/4184

GeorgeTailor commented 1 year ago

Almost definitely related to #4184

unlikely, the behaviour I described does not appear in 11.0, but only in 12.0. Although, I must admit this is not the only issue with CPU usage I've had, and one of them is similar to your case.

rome / tools