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

Using AsyncAdaptive #1088

Merged
merged 10 commits into from
Apr 12, 2023
Merged

Using AsyncAdaptive #1088

merged 10 commits into from
Apr 12, 2023

Conversation

TheAngryByrd
Copy link
Member

@TheAngryByrd TheAngryByrd commented Mar 24, 2023

WHAT

🤖 Generated by Copilot at 283c161

This pull request makes various parts of the FsAutoComplete project asynchronous and uses new libraries and utilities for better error handling and performance. It affects the code generation, analysis, and formatting features, as well as the LSP server and the code fixes modules. It also refactors some code and adds new dependencies to the paket.dependencies and src/FsAutoComplete.Core/paket.references files.

🤖 Generated by Copilot at 283c161

We're sailing on the async sea, with tasks and options galore
We're using FsToolkit and IcedTasks to make our code run more
We're switching to the background thread, to keep the main one free
We're heaving on the code gen service, one, two, three!

🔄🚀🐛

WHY

This converts the Adaptive LSP Server to use AsyncAval. This primarily comes from @krauthaufen's AsyncAdaptiveSketch with some tweaks to use IcedTasks and a Computation Expression.

  • Converts to using AsyncAval
  • Converts anything that was using sync calls to async. This change is invasive but most of it is pretty mechanical. (Doing let! instead of let, use async ce, etc)
  • A fix for the filesystem shim to use a flattened openedFilesWithChanges. Previously this would have to hit multiple locks in an AMap<string, AVal<VolatileFile>> to get the latest value, even though it should have been up to date. This now uses AMap<string, VolatileFile> which speeds this up considerable.
  • This also makes any calls into the FSharpChecker start on a new thread rather than use the threadpool. This keeps the server responsive while things happen in the background.
  • Moved some Activity RecordException helpers to Utils.

Some nice benefits I've noticed:

  • Server seems responsive pretty much all the time.
  • Find all references is rather quick

HOW

🤖 Generated by Copilot at 283c161

  • Add new dependencies for error handling and async utilities (link, link)
  • Modify the ICodeGenerationService interface to use async operations for tokenizing, parsing, and checking files (link, link, link)
  • Update the FSharpCompilerServiceChecker type to switch to a new thread before calling potentially long-running operations (link, link, link, link, link, link, link, link)
  • Refactor the Commands module to use async and asyncResult computation expressions and functions for parsing, checking, and tracing files and symbols (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Adapt the AbstractClassStubGenerator, RecordStubGenerator, and SymbolLocation modules to use the asyncOption computation expression and the new async version of the ICodeGenerationService interface (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Add a new function to the Async module in the Utils file to improve the performance and scalability of parallelizing async computations (link)
  • Add a new function to the SemanticConventions module in the Tracing module in the Utils file to improve the error handling and logging of tracing (link)
  • Modify the CodeFixes module and its submodules to use async and asyncResult computation expressions and functions for getting file versions and texts (link, link, link, link)
  • Replace the Async.Parallel function with the Async.parallel75 function in the Lsp module in the Conversions module in the LspHelpers file and the FSharpLspServer type in the FsAutoComplete.Lsp file (link, link, link)
  • Modify the FSharpLspServer type in the FsAutoComplete.Lsp file to use async and asyncResult computation expressions and functions for getting file versions, texts, and options (link, link, link, link)
  • Remove unused or duplicated functions from the Async module in the Common file (link, link)

@TheAngryByrd TheAngryByrd force-pushed the AsyncAdaptive branch 3 times, most recently from d3cbf24 to 61ae197 Compare March 26, 2023 14:19
paket.lock Outdated Show resolved Hide resolved
/// <remarks>
/// Since the task can be references multiple times in the dependency graph, it is important to cancel it only after there are no more references to it.
/// </remarks>
type internal RefCountingTaskCreator<'a>(create: CancellationToken -> Task<'a>) =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added comments in the module, please ask any clarifying questions. Also @krauthaufen please let me know if I've incorrectly done something here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I currently have a toddler-with-feaver-problem but once this is over I'll take some time to look over that...

src/FsAutoComplete.Core/AdaptiveExtensions.fs Outdated Show resolved Hide resolved
src/FsAutoComplete.Core/Commands.fs Show resolved Hide resolved
src/FsAutoComplete.Core/Utils.fs Outdated Show resolved Hide resolved
src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs Outdated Show resolved Hide resolved
src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs Outdated Show resolved Hide resolved
src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs Outdated Show resolved Hide resolved
@TheAngryByrd TheAngryByrd marked this pull request as ready for review March 26, 2023 23:36
@TheAngryByrd TheAngryByrd changed the title WIP: Using AsyncAdaptive Using AsyncAdaptive Mar 27, 2023
@TheAngryByrd TheAngryByrd force-pushed the AsyncAdaptive branch 4 times, most recently from c5fb1f3 to 283c161 Compare March 28, 2023 01:39
@TheAngryByrd TheAngryByrd force-pushed the AsyncAdaptive branch 7 times, most recently from 14e11f4 to 88731dc Compare April 6, 2023 01:55
@Krzysztof-Cieslak Krzysztof-Cieslak merged commit cb6f2ba into ionide:main Apr 12, 2023
@krauthaufen
Copy link

🎉🎉🎉🥳🥳🥳

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.

4 participants