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

Critic timeout #108

Open
pjcj opened this issue Jan 11, 2024 · 3 comments
Open

Critic timeout #108

pjcj opened this issue Jan 11, 2024 · 3 comments

Comments

@pjcj
Copy link

pjcj commented Jan 11, 2024

I have some large modules where critic times out and so no problems are reported.
The timeout is currently set to 25 seconds.
Would it be possible to make this configurable? This could be either per diagnostic or as a common value.

@bscan
Copy link
Owner

bscan commented Jan 11, 2024

Sure, this is a good idea. The 25 seconds is fairly arbitrary. I didn't want Critic processes hanging around forever, especially if they've hung or are otherwise not needed. Some related thoughts below:

A config parameter would be straightforward. Alternatively, I could add a "large document mode" for files > N lines that would automatically increase this value. The other piece worth considering is the delay between making changes and kicking off diagnostics. When editing, the Navigator doesn't run critic (or perl -c) on every change event, it waits until the user hasn't typed for 1 second. If we're in "large document mode", it might be worth waiting an extra second or so since it's going to take a long time anyway.

documents.onDidChangeContent((change) => {

Related, the Navigator doesn't kill processes that are no longer needed. If you are editing a file (with typing gaps of at least 1 second), the Navigator will keep spinning off subprocesses to run perl -c and perlcritic. These can pile up a bit and starve other processes of resources, especially on machines/containers with fewer cores. The 1 second wait prevents too many from running simultaneously, but killing processes is another option.

@pjcj
Copy link
Author

pjcj commented Jan 12, 2024

Thanks for thinking about this.

A "large document mode" is interesting and makes sense. Or perhaps values which are proportional to the size of the document? But in any case there still might be times the values need to be overridden somehow.

I currently have debounce_text_changes = 5000 in my neovim perlnavigator config which, I think, does something similar and won't run navigator more then once every five seconds. I'm happy to wait a little longer to get answers and use a little less power.

In the meantime I have increased the timeout locally and everything is working well.

@pjcj
Copy link
Author

pjcj commented Jul 22, 2024

This is probably mostly for my benefit, but may help others who have hit the same problem. To change the timeout I run:

perl -pi.bak -e 's/\b25000\b/120000/' ~/.local/share/nvim/mason/packages/perlnavigator/node_modules/perlnavigator-server/{src,out}/{formatting,diagnostics}.?s

Change the path to match where the code is installed for you.

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

2 participants