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

Remove Cache when non-existing file or file outside of workspace gets closed #1010

Merged
merged 2 commits into from
Sep 10, 2022

Conversation

Booksbaum
Copy link
Contributor

Currently caches for all files are kept forever (inside State).

That's ok, but in certain situations unnecessary and we can safely remove caches:

  • When a non-existing file gets closed (-> deleted file or untitled file (never existed in filesystem))
  • When a file outside the current workspace gets closed

It's nice to slightly reduce memory footprint for long sessions, but also fixes following cases:

  • VSCode shows old diags for a new untitled file with same name (even when language not set to F# in new untitled file...) -> clearing cache (or more precise: clearing diags) removes old diags

    • To reproduce:
      • In VSCode create new untitled document (Ctrl+N) (-> Untitled-1 if it's the only untitled doc)
      • set language to F# (-> F# script file)
      • write let foo = bar -> error "The value or constructor 'bar' is not defined."
      • close untitled document
      • create new untitled document (-> should have same name (Untitled-1))
      • Still shows error from old document
  • Iterating over all projects and their source files in State contains closed non-existing files (like untitled files) and external files -> not something to consider when doing something in FSAC, but are currently inside State.

    • Also an issue for tests: Same server, every testCase creates and untitled files (and closes it afterwards) -> State of 2nd testCase contains file & project for 1st testCase
    • I don't think that's currently an actual issue because nothing iterates over all files in all projects.
      But I'm looking into "Find all references" -> Sometimes want to find in all files -> iterate over all projects & their files. But this contains obsolete files too


This PR removes non-existing files and files outside workspace when they get closed (textDocument/didClose Notification) -> removed from State & diags get cleared for that file.


Some issues:

  • I think file might get reintroduced into State when a textDocument/didChange notification is still processed when textDocument/DidClose comes in
  • I think: A file outside of current workspace & outside of project might still be kept if file was previously part of project but removed from it during the current session.
    • Reason: Changes (at least file removals) inside fsproj aren't adopted by FSAC -> inside State the file is still part of a project and requires a restart to revaluate.

@baronfel
Copy link
Contributor

Great idea, and nice implementation. I agree with both of your caveats, but I don't think either is severe enough to prevent taking this,

@baronfel baronfel merged commit ef92608 into ionide:main Sep 10, 2022
@Booksbaum Booksbaum deleted the RemoveClosedFiles branch September 10, 2022 18:54
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

Successfully merging this pull request may close these issues.

2 participants