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

More performance tweaks adaptive #1028

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions paket.dependencies
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ nuget YoloDev.Expecto.TestSdk
nuget AltCover
nuget GitHubActionsTestLogger
nuget Ionide.LanguageServerProtocol
nuget Microsoft.Extensions.Caching.Memory
Copy link
Member Author

Choose a reason for hiding this comment

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

We can use stale results for completions to get it to be super quick. However the compiler's TryGetRecentTypeCheck only keeps type checks for the specific sourcetext, which won't really matter for us because we'll be doing incremental type checking on DidChange already


group Build
source https://api.nuget.org/v3/index.json
Expand Down
22 changes: 22 additions & 0 deletions paket.lock
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,33 @@ NUGET
System.Text.Encoding.CodePages (>= 4.0.1) - restriction: || (&& (== net7.0) (< net6.0)) (== netstandard2.0)
Microsoft.CodeCoverage (17.3) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net45)) (&& (== netstandard2.0) (>= netcoreapp1.0))
Microsoft.DotNet.PlatformAbstractions (3.1.6) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net5.0))
Microsoft.Extensions.Caching.Abstractions (6.0)
Microsoft.Extensions.Primitives (>= 6.0)
Microsoft.Extensions.Caching.Memory (6.0.1)
Microsoft.Extensions.Caching.Abstractions (>= 6.0)
Microsoft.Extensions.DependencyInjection.Abstractions (>= 6.0)
Microsoft.Extensions.Logging.Abstractions (>= 6.0)
Microsoft.Extensions.Options (>= 6.0)
Microsoft.Extensions.Primitives (>= 6.0)
Microsoft.Extensions.DependencyInjection.Abstractions (6.0)
Microsoft.Bcl.AsyncInterfaces (>= 6.0) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0)
System.Threading.Tasks.Extensions (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0)
Microsoft.Extensions.DependencyModel (6.0) - restriction: || (== net6.0) (== net7.0) (&& (== netstandard2.0) (>= net5.0))
System.Buffers (>= 4.5.1)
System.Memory (>= 4.5.4)
System.Runtime.CompilerServices.Unsafe (>= 6.0)
System.Text.Encodings.Web (>= 6.0)
System.Text.Json (>= 6.0)
Microsoft.Extensions.Logging.Abstractions (6.0.2)
System.Buffers (>= 4.5.1) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< net6.0)) (== netstandard2.0)
System.Memory (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< net6.0)) (== netstandard2.0)
Microsoft.Extensions.Options (6.0)
Microsoft.Extensions.DependencyInjection.Abstractions (>= 6.0)
Microsoft.Extensions.Primitives (>= 6.0)
System.ComponentModel.Annotations (>= 5.0) - restriction: || (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0)
Microsoft.Extensions.Primitives (6.0)
System.Memory (>= 4.5.4) - restriction: || (&& (== net6.0) (>= net461)) (&& (== net6.0) (< netcoreapp3.1)) (&& (== net7.0) (>= net461)) (&& (== net7.0) (< netcoreapp3.1)) (== netstandard2.0)
System.Runtime.CompilerServices.Unsafe (>= 6.0)
Microsoft.NET.StringTools (17.3.1) - copy_local: false
System.Memory (>= 4.5.5)
System.Runtime.CompilerServices.Unsafe (>= 6.0)
Expand Down Expand Up @@ -319,6 +340,7 @@ NUGET
System.Runtime.CompilerServices.Unsafe (>= 6.0)
System.CommandLine (2.0.0-beta4.22272.1)
System.Memory (>= 4.5.4) - restriction: || (&& (== net7.0) (< net6.0)) (== netstandard2.0)
System.ComponentModel.Annotations (5.0) - restriction: || (&& (== net6.0) (< netstandard2.1)) (&& (== net7.0) (< netstandard2.1)) (== netstandard2.0)
System.Configuration.ConfigurationManager (6.0)
System.Security.Cryptography.ProtectedData (>= 6.0)
System.Security.Permissions (>= 6.0)
Expand Down
52 changes: 47 additions & 5 deletions src/FsAutoComplete.Core/CompilerServiceInterface.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ open Ionide.ProjInfo.ProjectSystem
open FSharp.UMX
open FSharp.Compiler.EditorServices
open FSharp.Compiler.Symbols
open Microsoft.Extensions.Caching.Memory
open System
open FsToolkit.ErrorHandling



type Version = int

Expand All @@ -26,6 +31,9 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =

let entityCache = EntityCache()

let mutable lastCheckResults: IMemoryCache =
new MemoryCache(MemoryCacheOptions(SizeLimit = Nullable<_>(20L)))

let checkerLogger = LogProvider.getLoggerByName "Checker"
let optsLogger = LogProvider.getLoggerByName "Opts"

Expand Down Expand Up @@ -227,14 +235,22 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =
member __.ScriptTypecheckRequirementsChanged =
scriptTypecheckRequirementsChanged.Publish

/// This function is called when the entire environment is known to have changed for reasons not encoded in the ProjectOptions of any project/compilation.
member _.ClearCaches() =
Copy link
Member Author

Choose a reason for hiding this comment

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

Useful for major project changes like git checkout

let oldlastCheckResults = lastCheckResults
lastCheckResults <- new MemoryCache(MemoryCacheOptions(SizeLimit = Nullable<_>(20L)))
oldlastCheckResults.Dispose()
checker.InvalidateAll()
checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients()

member __.ParseFile(fn: string<LocalPath>, source, fpo) =
checkerLogger.info (Log.setMessage "ParseFile - {file}" >> Log.addContextDestructured "file" fn)

let path = UMX.untag fn
checker.ParseFile(path, source, fpo)

member __.ParseAndCheckFileInProject(filePath: string<LocalPath>, version, source: ISourceText, options) =
async {
asyncResult {
let opName = sprintf "ParseAndCheckFileInProject - %A" filePath

checkerLogger.info (Log.setMessage "{opName}" >> Log.addContextDestructured "opName" opName)
Expand All @@ -255,32 +271,58 @@ type FSharpCompilerServiceChecker(hasAnalyzers) =
>> Log.addContextDestructured "errors" (List.ofArray p.Diagnostics)
)

return ResultOrString.Error(sprintf "Check aborted (%A). Errors: %A" c parseErrors)
return! ResultOrString.Error(sprintf "Check aborted (%A). Errors: %A" c parseErrors)
| FSharpCheckFileAnswer.Succeeded (c) ->
checkerLogger.info (
Log.setMessage "{opName} completed successfully"
>> Log.addContextDestructured "opName" opName
)

return Ok(ParseAndCheckResults(p, c, entityCache))
let r = ParseAndCheckResults(p, c, entityCache)

let ops =
MemoryCacheEntryOptions()
.SetSize(1)
.SetSlidingExpiration(TimeSpan.FromMinutes(2.))
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if a time is really needed.


return lastCheckResults.Set(filePath, r, ops)
with ex ->
return ResultOrString.Error(ex.ToString())
return! ResultOrString.Error(ex.ToString())
}

member _.TryGetLastCheckResultForFile(file: string<LocalPath>) =
let opName = sprintf "TryGetLastCheckResultForFile - %A" file

checkerLogger.info (
Log.setMessage "{opName}" >> Log.addContextDestructured "opName" opName

)

match lastCheckResults.TryGetValue<ParseAndCheckResults>(file) with
| (true, v) -> Some v
| _ -> None

member __.TryGetRecentCheckResultsForFile(file: string<LocalPath>, options, source: ISourceText) =
let opName = sprintf "TryGetRecentCheckResultsForFile - %A" file

checkerLogger.info (
Log.setMessage "{opName} - {hash}"
>> Log.addContextDestructured "opName" opName
>> Log.addContextDestructured "hash" (source.GetHashCode() |> int)

)

let options = clearProjectReferences options

let result =
checker.TryGetRecentCheckResultsForFile(UMX.untag file, options, sourceText = source, userOpName = opName)
|> Option.map (fun (pr, cr, _) -> ParseAndCheckResults(pr, cr, entityCache))
|> Option.map (fun (pr, cr, version) ->
checkerLogger.info (
Log.setMessage "{opName} - got results - {version}"
>> Log.addContextDestructured "version" version
)

ParseAndCheckResults(pr, cr, entityCache))

checkerLogger.info (
Log.setMessage "{opName} - {hash} - cacheHit {cacheHit}"
Expand Down
10 changes: 6 additions & 4 deletions src/FsAutoComplete.Core/FileSystem.fs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ type NamedText(fileName: string<LocalPath>, str: string) =
// because we know there are lines after the first line
let firstLine = (x :> ISourceText).GetLineString(m.StartLine - 1)

builder.AppendLine(firstLine.Substring(m.StartColumn))
builder.AppendLine(firstLine.Substring(Math.Min(firstLine.Length, m.StartColumn)))
|> ignore<System.Text.StringBuilder>

// whole intermediate lines, including newlines
Expand All @@ -155,7 +155,7 @@ type NamedText(fileName: string<LocalPath>, str: string) =
// final part, potential slice, so we do not include the trailing newline
let lastLine = (x :> ISourceText).GetLineString(m.EndLine - 1)

builder.Append(lastLine.Substring(0, m.EndColumn))
builder.Append(lastLine.Substring(0, Math.Min(lastLine.Length, m.EndColumn)))
|> ignore<System.Text.StringBuilder>

Ok(builder.ToString())
Expand Down Expand Up @@ -260,8 +260,8 @@ type NamedText(fileName: string<LocalPath>, str: string) =
member x.ModifyText(m: FSharp.Compiler.Text.Range, text: string) : Result<NamedText, string> =
result {
let startRange, endRange = x.SplitAt(m)
let! startText = x[startRange]
let! endText = x[endRange]
let! startText = x[startRange] |> Result.mapError (fun x -> $"startRange -> {x}")
let! endText = x[endRange] |> Result.mapError (fun x -> $"endRange -> {x}")
let totalText = startText + text + endText
return NamedText(x.FileName, totalText)
}
Expand Down Expand Up @@ -355,6 +355,8 @@ type VolatileFile =
Lines: NamedText
Version: int option }

member this.FileName = this.Lines.FileName

/// <summary>Updates the Lines value</summary>
member this.SetLines(lines) = { this with Lines = lines }

Expand Down
8 changes: 6 additions & 2 deletions src/FsAutoComplete.Core/ParseAndCheckResults.fs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,11 @@ type ParseAndCheckResults

let residue = longName.PartialIdent

logger.info (
Log.setMessage "TryGetCompletions - lineStr: {lineStr}"
>> Log.addContextDestructured "lineStr" lineStr
)

logger.info (
Log.setMessage "TryGetCompletions - long name: {longName}"
>> Log.addContextDestructured "longName" longName
Expand Down Expand Up @@ -581,9 +586,8 @@ type ParseAndCheckResults
| Some k when k.Kind = Operator -> return None
| Some k when k.Kind = Keyword -> return None
| _ ->

let results =
checkResults.GetDeclarationListInfo(Some parseResults, pos.Line, lineStr, longName, getAllSymbols)
checkResults.GetDeclarationListInfo(Some parseResults, pos.Line, lineStr, longName, getSymbols)

let getKindPriority =
function
Expand Down
1 change: 1 addition & 0 deletions src/FsAutoComplete.Core/paket.references
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ System.Reflection.Metadata
Microsoft.Build.Utilities.Core
Ionide.LanguageServerProtocol
Ionide.KeepAChangelog.Tasks
Microsoft.Extensions.Caching.Memory
32 changes: 18 additions & 14 deletions src/FsAutoComplete/CodeFixes/ResolveNamespace.fs
Original file line number Diff line number Diff line change
Expand Up @@ -76,22 +76,26 @@ let fix
ns

let lineStr =
let whitespace = String.replicate ctx.Pos.Column " "
let whitespace =
let column =
// HACK: This is a work around for inheriting the correct column of the current module
// It seems the column we get from FCS is incorrect
let previousLine = docLine - 1
let insertionPointIsNotOutOfBoundsOfTheFile = docLine > 0

let theThereAreOtherOpensInThisModule () =
text.GetLineString(previousLine).Contains "open "

if insertionPointIsNotOutOfBoundsOfTheFile && theThereAreOtherOpensInThisModule () then
text.GetLineString(previousLine).Split("open") |> Seq.head |> Seq.length // inherit the previous opens whitespace
else
ctx.Pos.Column

String.replicate column " "

$"%s{whitespace}open %s{actualOpen}\n"

let edits =
[| yield insertLine docLine lineStr
if
text.GetLineCount() < docLine + 1
&& text.GetLineString(docLine + 1).Trim() <> ""
then
yield insertLine (docLine + 1) ""
if
(ctx.Pos.Column = 0 || ctx.ScopeKind = ScopeKind.Namespace)
&& docLine > 0
&& not (text.GetLineString(docLine - 1).StartsWith "open")
then
yield insertLine (docLine - 1) "" |]
let edits = [| yield insertLine docLine lineStr |]

{ Edits = edits
File = file
Expand Down
Loading