JetBrains / resharper-fsharp

F# support in JetBrains Rider
Apache License 2.0
327 stars 57 forks source link

Too many parsing requests for the active file on every edit/focus. #492

Open safesparrow opened 1 year ago

safesparrow commented 1 year ago

Parsing can take ~10-20% of time to check files/projects. I think some of the requests are not necessary. This compounded by lack of parsing caching in FCS means there is a lot of unnecessary work happening all the time in the IDE.

I'm running Rider with a locally built FCS. I added a breakpoint here: https://github.com/JetBrains/fsharp/blob/35e742b334353de45b11d1d5c4cfecb02e6e58fe/src/Compiler/Driver/ParseAndCheckInputs.fs#L428 and can see that the parsing happens more often than I would expect when editing/focusing on files.

Some extract showing the parsing events. Every ParseInput(fileName) means a single call to the above method. The other lines reflect user actions:

1. Change cursor position in the editor
ParseInput(Sandbox.fs)
ParseInput(Sandbox.fs)
2. Change cursor position again
ParseInput(Sandbox.fs)
ParseInput(Sandbox.fs)
3. Type a single character
ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
ParseInput(Sandbox.fs)
ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
ParseInput(Sandbox.fs)
4. Click outside the Rider window
ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
5. Click again inside the editor
ParseInput(Sandbox.fs)
ParseInput(Sandbox.fs)
6. Type a single character
ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)

The following is a result of these 3 steps:

  1. focus on editor
  2. type let x = 1
  3. leave focus
    ParseInput(Sandbox.fs)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(Sandbox.fs)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(Sandbox.fs)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(Sandbox.fs)
    ParseInput(Sandbox.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)
    ParseInput(C:\projekty\fantomas\src\Fantomas\AssemblyInfo.fsi)

It does feel like there are a few too many calls to parse a file. In particular I can see the following issues:

  1. Cursor position change triggers two parsing requests.
  2. Typing a single character causes four parsing requests.
  3. Clicking outside the window triggers a parsing request.
  4. There are separate requests for Sandbox.fs and AssemblyInfo.fsi.
  5. Subsequent calls for AssemblyInfo.fsi do not hit the parsing cache. I can see the right entry in the cache, but parsing still occurs - see the screenshot below. This one is probably to be fixed in FCS, but still relevant: image

More context for 5.: I found that one of the reasons caching doesn't work is that the two caching keys, while otherwise identical, have a different set of FSharpParsingOptions.ConditionalDefines. One of them has these two extra defines: EDITING ; COMPILED at the top (note that COMPILED also appears at the end of the list in both.

image image

Is this set of defines controlled by Rider? If so, could the sets in both calls be aligned?

auduchinok commented 1 year ago

At least some of the additional parsing is expected: during calculating completion suggestions we insert fake identifiers that allow us to get reparsed nodes as if there was a name typed in already, and it helps a lot and needed for many of completion features. These requests are not expected to be cached anywhere or to influence the project caches and it was the reason I added the cache/noCache option to FCS some time ago. We can analyze it and see if they are excessive, but they shouldn't matter if we want to improve a project type checking time.

I found that one of the reasons caching doesn't work is that the two caching keys, while otherwise identical, have a different set of FSharpParsingOptions.ConditionalDefines Is this set of defines controlled by Rider? If so, could the sets in both calls be aligned?

This one looks like a bug if the file is parsed in the context of a project, and not in the sandbox (used in completion and other features).

safesparrow commented 1 year ago

With the latest FCS I can see that non-active files are only parsed once after project load and cached correctly.

I'm looking at remaining excessive requests. I can see that the two requests for the active file on every cursor position change are seemingly identical and could be cached, except cache is set to false. I dumped the contents of the request and it is identical both times:

{
"FileName":"Sandbox.fs",
"Ident":false,
"Options":["EDITING","COMPILED","NETCOREAPP3_1_OR_GREATER","NETCOREAPP3_0_OR_GREATER","NETCOREAPP2_2_OR_GREATER","NETCOREAPP2_1_OR_GREATER","NETCOREAPP2_0_OR_GREATER","NETCOREAPP1_1_OR_GREATER","NETCOREAPP1_0_OR_GREATER","NET7_0_OR_GREATER","NET6_0_OR_GREATER","NET5_0_OR_GREATER","NETCOREAPP","NET7_0","NET","DEBUG","TRACE","COMPILED"],
"Suggest":false,
"Text":"module FSharp.A\n\nlet x = 3\nlet x : CShaReSharperFSharpRulezzzrp.Y = failwith \"\"\ntype X = X of int              ",
"UserOpName":"Unknown"
}

Are these two requests somehow different in a way I can't see? Could they be de-duplicated?

EDIT: Sorry, I'm being stupid, I didn't dump most of the FSharpParsingOptions object, which I guess is likely to differ. Will check that.

EDIT2: I made sure to dump all the options, and the two requests are still identical:

Request ``` { "FileName": "Sandbox.fs", "Ident": false, "Options": { "SourceFiles": [ "C:\\Users\\janus\\RiderProjects\\DoubleCheckingRepro\\FSharp\\obj\\Debug\\net7.0\\.NETCoreApp,Version=v7.0.AssemblyAttributes.fs", "C:\\Users\\janus\\RiderProjects\\DoubleCheckingRepro\\FSharp\\obj\\Debug\\net7.0\\FSharp.AssemblyInfo.fs", "C:\\Users\\janus\\RiderProjects\\DoubleCheckingRepro\\FSharp\\A.fs", "C:\\Users\\janus\\RiderProjects\\DoubleCheckingRepro\\FSharp\\B.fs", "C:\\Users\\janus\\RiderProjects\\DoubleCheckingRepro\\FSharp\\C.fs" ], "ApplyLineDirectives": false, "ConditionalDefines": [ "EDITING", "COMPILED", "NETCOREAPP3_1_OR_GREATER", "NETCOREAPP3_0_OR_GREATER", "NETCOREAPP2_2_OR_GREATER", "NETCOREAPP2_1_OR_GREATER", "NETCOREAPP2_0_OR_GREATER", "NETCOREAPP1_1_OR_GREATER", "NETCOREAPP1_0_OR_GREATER", "NET7_0_OR_GREATER", "NET6_0_OR_GREATER", "NET5_0_OR_GREATER", "NETCOREAPP", "NET7_0", "NET", "DEBUG", "TRACE", "COMPILED" ], "DiagnosticOptions": { "WarnLevel": 3, "GlobalWarnAsError": false, "WarnOff": [ 3390 ], "WarnOn": [ 1182 ], "WarnAsError": [ 3239 ], "WarnAsWarn": [], "CheckXmlDocs": false }, "LangVersionText": "default", "IsInteractive": false, "IndentationAwareSyntax": null, "CompilingFSharpCore": false, "IsExe": false }, "Suggest": false, "Text": "module FSharp.A\n\nlet x = 3\nlet x : CSharp.Y = faReSharperFSharpRulezzzilwith \"\"\ntype X = X of int ", "UserOpName": "Unknown" } ```

Code used (added in https://github.com/dotnet/fsharp/blob/90abb7f276b080903978fbbd34e3633c194cc93a/src/Compiler/Service/FSharpCheckerResults.fs#L2420):

    let parseFile
        (
            sourceText: ISourceText,
            fileName,
            options: FSharpParsingOptions,
            userOpName: string,
            suggestNamesForErrors: bool,
            identCapture: bool
        ) =
...
        let data =
            {|
              Text = sourceText.GetSubTextString(0, sourceText.Length)
              FileName = fileName
              Options = options
              UserOpName = userOpName
              Suggest = suggestNamesForErrors
              Ident = identCapture
              |}
        File.WriteAllText(path, Newtonsoft.Json.JsonConvert.SerializeObject(data, Formatting.Indented))
safesparrow commented 1 year ago

Slightly orthogonal, but partly driven by the above discrepancy in defines:

Typing a single character causes 5 requests (2 Sandbox requests + 3 A.fs requests), and none of them are cached. I'm attaching the requests below: Parsing requests typing.zip

auduchinok commented 1 year ago

@safesparrow Thanks for your analysis, I'm going to look into it.

NinoFloris commented 4 months ago

Writing F# in Rider on my M1 is still quite choppy... Even a single file project stutters during typing, tab insertion and reformatting etc.

auduchinok commented 4 months ago

Writing F# in Rider on my M1 is still quite choppy... Even a single file project stutters during typing, tab insertion and reformatting etc.

@NinoFloris I don't think what you see could be caused by parsing requests reported here, as they have never been seen in the snapshots we've analyzed (unlike some other issues that we've fixed or are trying to fix). Could you please capture a backend performance snapshot and file a separate issue, so we could look into it?