styler too slow for large script and blocks lsp

[renkun-ken]

I'm editing a script of more than 1500 lines of code. I timed the formatting by syste.time(styler::style_file(file)):

ℹ 
────────────────────────────────────────
Status  Count   Legend 
✔       0       File unchanged.
ℹ       1       File changed.
✖       0       Styling threw an error.
────────────────────────────────────────
Please review the changes carefully!
   user  system elapsed 
 46.542   0.059  46.607 

If I execute command Format document, then languageserver totally is stuck at formatting the file and stops providing other functionalities, looks like the formatting task completely blocks the langaugeserver.

I also enable formatOnPaste, if I copy/paste a large chunk of code (hundreds of lines) to the current document, then range formatting is triggered, which also blocks languageserver.

[renkun-ken]

Related: r-lib/styler#558

[randy3k]

I think Format Document should be a blocking action though to ensure user changes are not lost. There may be nothing we could do at this moment.

[lorenzwalthert]

Also we hope that we can cache on an expression level in r-lib/styler#570 which will speed up things when a file was partly edited after we implemented caching on the whole input (r-lib/styler#538) in v1.2.0.

[lorenzwalthert]

If this causes a real headache and many upvote r-lib/styler#570, chances are that it is implemented more quickly.

[lorenzwalthert]

We have now implemented r-lib/styler#570 in the devel version of styler, meaning:

1. initial styling is as slow as before.
2. styling a file that has previously been styled (and typically only the minority of the tokens in that file changed) is now quite a bit quicker.
3. using https://github.com/lorenzwalthert/precommit will also be quicker because of 2).

Just wanted to let you know if you want to test and provide feedback. Also, we removed a bug in in the caching feature that could be detrimental (changing tokens, see r-lib/styler#584), so if you use the GitHub version, make sure to get the latest version of styler with remotes::install_github("r-lib/styler").

[renkun-ken]

@lorenzwalthert I'm using the latest styler master branch to test the caching. Is there anything I should do to enable it? I've tried styling the 1500+ line file which took more than 40 seconds. But styling it again without any modification still took nearly the same amount of time.

[lorenzwalthert]

What does cache_info() show? Is the R.cache package installed? Did you restart R after installation of the GitHub version? Have you styled with styler::style_file()? I just tried styler::style_file() and it worked for me in RStudio and via the RStudio Addin.

[renkun-ken]

Thanks! It is caused by I didn't install R.cache. Now it works and repeated formatting file with minor changes looks much faster.

I notice that the first time use of R.cache it will ask user where to create the cache directory. However, languageserver is started non-interactively. Do we have to do anything about it in languageserver?

[renkun-ken]

Unfortunately, languageserver does not use styler::style_file() to perform styling on file save because it is requested before file is saved, so only a character vector of lines is supplied, and it is not associated to the original file. As a result, each formatting done in languageserver looks like it is working with a new content.

[lorenzwalthert]

Ok, good to know. R.cache is only a suggested package because it has a like 3 recursive dependencies and we wanted to make the usage of the cache flexible. Maybe we should actually make it an Imports:. Do you have a chance to check on your side?
The caching should also work for any API, i.e. text as well as file and cache is shared. Note that what cached is the pretty output. So if you supply 1+1 twice, the cache won't be used. Only when you use 1 + 1, styler will return the input immediately.

[renkun-ken]

@lorenzwalthert Thanks for the information. It works quite well for me now.

[lorenzwalthert]

Do you have a chance to check on your side?

I don't mean you personally, but languageserver.

[renkun-ken]

Yes, languageserver formatting works with styler caching. The only limitation when styling for my script is that it only caches top-level expressions. Some of my scripts contain large chunks of while or if which contains hundreds of lines inside the block.

[lorenzwalthert]

Ok. Yes, these are known limitations. So it's an encouragement to program functions with narrow scope 😄.

But the main problem I worry is that with the current version of languageserver and styler, 99% of users will never benefit from caching because they won't read this issue and realise they need to install {R.cache} manually. See also r-lib/styler#586.

[renkun-ken]

Maybe we should add some documentation for this.

[lorenzwalthert]

Ok, so with r-lib/styler#586, {R.cache} will be moved from Suggests to Imports. However, the problem persists: The user needs to authorise {R.cache} once to create a permanent directory in an interactive session. Otherwise, code won't be cached across R sessions, only within. I think this merits mentioning in the README, in particular since you already have a section on {styler}.

[renkun-ken]

@lorenzwalthert The README has been updated to reflect your work on styler caching. Closing this.

[lorenzwalthert]

Cool, thanks. A detail: The permanent cache will only be created with a command like R.cache::getCachePath() if you are running it in interactive R session. This is precisely the reason why when you run styler through {languageserver}, you will never be asked to create a permanent cache, but when you run {styler} interactively, you will. Hence, I'd adapt one of the sentences and highlight that you must call it in an interactive session and also mention styler::cache_info() that shows the path so people can double check if a permanent directory is used or not.

[renkun-ken]

Thanks for your detailed information. 8f4d281 makes it clear as you suggest.

[lorenzwalthert]

Perfect, thanks. Also note that we are experiencing some problematic behavior with caching and we suggest to use v1.3.1 or even the current GitHub version for now. This will be fixed by end of February with v1.3.2 because the CRAN team has requested a maintenance update that is unrelated to the caching feature. I don't think this needs to be mentioned in the README.