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

Reduce project option duplication, reducing memory usage. #1147

Merged
merged 1 commit into from
Jul 22, 2023
Merged
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
56 changes: 25 additions & 31 deletions src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -59,28 +59,22 @@ type AdaptiveWorkspaceChosen =
| Projs of amap<string<LocalPath>, DateTime>
| NotChosen

type LoadedProjectFile =
{ ProjectOptions: Types.ProjectOptions
FSharpProjectOptions: FSharpProjectOptions
LanguageVersion: LanguageVersionShim }

type LoadedScriptFile =
[<CustomEquality; NoComparison>]
type LoadedProject =
{ FSharpProjectOptions: FSharpProjectOptions
LanguageVersion: LanguageVersionShim }

type LoadedProject =
| LoadedProjectFile of LoadedProjectFile
| LoadedScriptFile of LoadedScriptFile
interface IEquatable<LoadedProject> with
member x.Equals(other) =
x.FSharpProjectOptions = other.FSharpProjectOptions

member x.FSharpProjectOptions =
match x with
| LoadedProjectFile p -> p.FSharpProjectOptions
| LoadedScriptFile p -> p.FSharpProjectOptions
override x.GetHashCode() = x.FSharpProjectOptions.GetHashCode()

member x.LanguageVersion =
match x with
| LoadedProjectFile p -> p.LanguageVersion
| LoadedScriptFile p -> p.LanguageVersion
override x.Equals(other) =
match other with
| :? LoadedProject as other -> (x :> IEquatable<_>).Equals other
| _ -> false
Comment on lines +63 to +77
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 don't really need to keep ProjectOptions so condensing this type. Also makes it use FSharpProjectOptions as its equality.


member x.SourceFiles = x.FSharpProjectOptions.SourceFiles
member x.ProjectFileName = x.FSharpProjectOptions.ProjectFileName
Expand Down Expand Up @@ -793,9 +787,10 @@ type AdaptiveFSharpLspServer
checker.ClearCaches() // if we got new projects assume we're gonna need to clear caches

let options =
projectOptions
|> List.map (fun o ->
let fso = FCS.mapToFSharpProjectOptions o projectOptions
let fsharpOptions = projectOptions |> FCS.mapManyOptions |> Seq.toList
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the main change. We had this in Ionide.ProjInfo but weren't using it. This prevent recreation of referenced types.


List.zip projectOptions fsharpOptions
|> List.map (fun (projectOption, fso) ->

let langversion = LanguageVersionShim.fromFSharpProjectOptions fso

Expand All @@ -806,14 +801,14 @@ type AdaptiveFSharpLspServer
Stamp = fso.Stamp |> Option.orElse (Some DateTime.UtcNow.Ticks)
ProjectId = fso.ProjectId |> Option.orElse (Some(Guid.NewGuid().ToString())) }

{ ProjectOptions = o
FSharpProjectOptions = fso
LanguageVersion = langversion })
{ FSharpProjectOptions = fso
LanguageVersion = langversion },
projectOption)

options
|> List.iter (fun loadedProject ->
let projectFileName = loadedProject.FSharpProjectOptions.ProjectFileName
let projViewerItemsNormalized = ProjectViewer.render loadedProject.ProjectOptions
|> List.iter (fun (loadedProject, projectOption) ->
let projectFileName = loadedProject.ProjectFileName
let projViewerItemsNormalized = ProjectViewer.render projectOption

let responseFiles =
projViewerItemsNormalized.Items
Expand All @@ -833,9 +828,9 @@ type AdaptiveFSharpLspServer
let ws =
{ ProjectFileName = projectFileName
ProjectFiles = responseFiles
OutFileOpt = Option.ofObj loadedProject.ProjectOptions.TargetPath
OutFileOpt = Option.ofObj projectOption.TargetPath
References = references
Extra = loadedProject.ProjectOptions
Extra = projectOption
ProjectItems = projViewerItemsNormalized.Items
Additionals = Map.empty }

Expand All @@ -846,7 +841,7 @@ type AdaptiveFSharpLspServer

notifications.Trigger(not, CancellationToken.None)

return options |> List.map LoadedProject.LoadedProjectFile
return options |> List.map fst
}

/// <summary>
Expand Down Expand Up @@ -1133,9 +1128,8 @@ type AdaptiveFSharpLspServer
opts |> scriptFileProjectOptions.Trigger

return
LoadedScriptFile
{ FSharpProjectOptions = opts
LanguageVersion = LanguageVersionShim.fromFSharpProjectOptions opts }
{ FSharpProjectOptions = opts
LanguageVersion = LanguageVersionShim.fromFSharpProjectOptions opts }
}

return file, Option.toList projs
Expand Down