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

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

Open
safesparrow opened this issue Mar 5, 2023 · 6 comments
Open

Comments

@safesparrow
Copy link

safesparrow commented Mar 5, 2023

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
Copy link
Collaborator

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
Copy link
Author

safesparrow commented Mar 18, 2023

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
Copy link
Author

safesparrow commented Mar 18, 2023

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
Copy link
Collaborator

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

@NinoFloris
Copy link

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
Copy link
Collaborator

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?

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