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

Very high CPU usage when editing fs file #975

Closed
Booksbaum opened this issue Jul 25, 2022 · 12 comments
Closed

Very high CPU usage when editing fs file #975

Booksbaum opened this issue Jul 25, 2022 · 12 comments

Comments

@Booksbaum
Copy link
Contributor

With the new Ionide version (7.0.0) and new FSAC version (0.56.0) (included in Ionide):
Extreme CPU usage when editing code (up to 100% over all cores...) in fs file (-> inside a project, not Script files)

  • Higher CPU usage when file is higher up in F# file order in project
  • Higher CPU usage when file is newly opened

Repro steps:
In FsAutoComplete open src\FsAutoComplete.Core\TipFormatter.fs, place cursor somewhere and just type whatever on the keyboard. CPU skyrockets and typing in VSCode becomes unresponsive.
After not typing anything for a short while everything gets back to normal.



Location of high CPU usage is CompilerServiceInterface.ParseAndCheckFileInProject (-> FSharpChecker.ParseAndCheckFileInProject). It's expected to spend lot of time checking F# file with FCS, but not that much...


Digging deeper:

Commands.CheckFileAndAllDependentFilesInAllProjects not only checks current file, but also files after current F# file inside project (and files before when first open)
-> matches higher usage when file is higher up in project file order; and when file is first opened

  • Uncommenting everything except checking current file seems to result in a more reasonable CPU utilization
    (though I think still higher (or with higher peaks?) than previous FSAC/Ionide version?)
  • Previous FSAC/Ionide release just checked current file (-> no CPU issues)
  • I think same issue as Disable cross-project type checking after edit #971 .... but that PR just disabled checking of other projects (performance was probably even worse then....)



  • OS: Win 11 x64
  • dotnet --version: 6.0.302
@razzmatazz
Copy link
Contributor

I had this issue happening on non-vs code client (emacs) on M1 mac, which I thought was related to ARM arch but could be related to this, too:

@Booksbaum
Copy link
Contributor Author

Booksbaum commented Jul 25, 2022

I think they are slightly different:

with the latest fsac there appears to be an excessive CPU usage running for a very long time (10 mins +) ; the intectiveness of LSP server is significantly reduced too
[...]
and this is running even if I am not doing anything in the editor for a long time (5mins +)

I just have the issue while typing. And once stopped the CPU calms down reasonable quick (seconds, not minutes).

Though here it might be because of different CPU architectures? Or maybe different solution/project sizes? (Though razzmatazz/csharp-language-server seems to be smaller than FSAC (which I used for tests). But maybe different F# usage that caused different behaviour).



Further tests for similarities:

  • Do you have extreme CPU utilization in every F# file (including Script files) or just in .fs in projects? (Script files work fine for me)
  • Does position of file in project matter for you?
    Example from FsAutoComplete.Core.fsproj:
    <ItemGroup>
      <!-- [...] -->
      <Compile Include="TipFormatter.fs" />
      <!--  [... 18 other, large fs files] -->
      <Compile Include="Commands.fs" />
    </ItemGroup>
    • Editing Commands.fs is doable -- high usage, but not extreme Very high usage, but slightly less extreme than in TipFormatter....
    • Editing TipFormatter.fs isn't possible: extreme usage after some characters

@Krzysztof-Cieslak
Copy link
Member

Previous FSAC/Ionide release just checked current file

On the main process. However, checking other files from the project WAS happening in the Background Service. However, due to some architectural changes in FCS, we don't need to do multi-process stuff anymore, so we moved back all the type checking to the main process.

@Booksbaum
Copy link
Contributor Author

On the main process. However, checking other files from the project WAS happening in the Background Service.

hm, interesting

Using older Ionide (6.0.6) the background service exist -- but isn't doing anything. There doesn't seem to be any checking happening?
I can see a spike in normal FSAC process while editing. CPU load is then about the same as now with just checking the current file (no dependents & dependencies (-> uncommented)). But the background service doesn't seems to get triggered.

So it seems the difference is: previously background checking/checking of related files didn't happen?...

Another sign: Diagnostics after edit were just published for current file, but not for other files (in new FSAC version they are)

@Krzysztof-Cieslak
Copy link
Member

So it seems the difference is: previously background checking/checking of related files didn't happen?...

There might have been bugs in stuff related to background service, but in principle, we always wanted to do checking (and yes, publishing diagnostics is the main user-visible result of it) of other files (that are below the current file in fsrpoj ) on edit.

For what it's worth - I can reproduce the problem when I type really fast/keep the single key pressed for a while. Single character edits are not causing a huge spike (ofc, there's some CPU usage visible, but that's expected), which suggests that it's some kind of problem with cancellation (or maybe we should add some debouncing)

@Krzysztof-Cieslak
Copy link
Member

Hey, @Booksbaum and @razzmatazz - could you try using FSAC from this branch - #977 ?

@Booksbaum
Copy link
Contributor Author

It's way better -- but still way too high


The good:
It doesn't immediately jump (close) to 100% CPU any more and basically blocks VSCode and makes the OS and running applications stutter.
And it maintains a somewhat reasonable CPU utilization for way longer (~20-30% -- similar to previous Ionide version).
And when CPU goes up, it seems to calm down faster than before.
Writing code is now possible.

The bad:
CPU is still quite high. After some continuous writing the CPU reaches up to 90% CPU usage (and CPU fan cranks up...).
It doesn't seem to impact low-impact side-applications any more (like watching a video) -- but I don't think running something more intense isn't practical

The ugly:
I can still make input in VSCode stutter -- but it's just a very minor stutter. Not enough to make writing impossible, but enough that it feels wrong.



-> It's WAY better than before -- but still way too much CPU usage

@Krzysztof-Cieslak
Copy link
Member

It's WAY better than before -- but still way too much CPU usage

I mean, the F# compiler being fairly slow/performance intensive to use is not something I can fix it right now ;-) If the fix brings it back to usable state/similar utilization to previous versions, it sounds good to me.

@Booksbaum
Copy link
Contributor Author

Booksbaum commented Jul 26, 2022

If the fix brings it back to usable state/similar utilization to previous versions, it sounds good to me.

It's still only similar and really usable when no other files than the current one gets checked. (Which is ok for me)

-> maybe as compromise/short-term-solution we could introduce a setting to disable checking other files? (here and for startup here (large spike when opening a new document for some seconds)).

Then everyone can decide if diags for other files or less CPU is more important. And also depending on the size of the solution.


I mean, the F# compiler being fairly slow/performance intensive to use is not something I can fix it right now ;-)

ohh...no alternative F# compiler up your sleeve? :D



But I don't think it's just the F# compiler. Yes it's not exactly great performance wise -- but it's acceptable when it's just checking one file at a time.
So if whe just check one file at a time that should not spike the CPU that much. Seems to indicate there's more than one checking happening at the same time. Checks in Commands.CheckFileAndAllDependentFilesInAllProjects are sequential. So why CPU increase?

I guess multiple checkChangedFile run into each other (one gets started and executed while an older one is still checking (they get just started and forget)).
Short test to support this: Change fn in Debouncer to block till finished (prepend do! to fn arg.Value in Debouncer and remove Async.Start from checkChangedFile. Edit: Changes are in Booksbaum@7cba4e8). Now FSAC has the expected performance of just one file check at a time

-> We should try to prevent multiple parallel checks

Issue with solution above: Checking of current file gets delayed because some other file in project is still checking -> less responsive for user

Then probably extract checking of other-than-current-working-file and only check them when there's no working-file-check active?

Or just kill old checkChangedFile when a new checkChangedFile gets started. (can that leave anything in an invalid state?)

@razzmatazz
Copy link
Contributor

razzmatazz commented Jul 26, 2022

I am not sure this is very much related to the discussion but lsp 3.17 now has “diagnostic pull model”, where the editor would pull diagnostics for the files it is displaying instead of the server doing the computation and pushing diagnostics proactively which I suppose is what is happening here.

not sure about 3.17 support in vscode though

@Booksbaum
Copy link
Contributor Author

VSCode seems to support pull diagnostics

But this doesn't solve our problem as we still have/want to analyze all related files. textDocument/diagnostics "just" tells server for what document diagnostics are currently most important (-> current working document). (We already have a priority with textDocument/didChange notification. Though it also notifies about changes in other-than-current-working file (like for refactors over multiple files), it usually is a reasonable indication of file to prioritize (-> most didChange notification are for file the user currently writes))
Diagnostics for other related files should then still be pushed to client (-> notify user: hey your edit broke/fixed something in another file) (LSP even supports sending diags for related documents with pull diags -> again have to analyze all related files)

-> pull diags are more about priority than load reduction

@razzmatazz
Copy link
Contributor

razzmatazz commented Aug 20, 2022

Hey, @Booksbaum and @razzmatazz - could you try using FSAC from this branch - #977 ?

didn't test the branch yet, because I couldn't build it on m1 mac due to fake breakage on macos/arm64 (?) but the latest fsac as of v0.56.2 fixes the performance/cpu hog issue for me

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

3 participants