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.

Sign up for GitHub

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

Fixed .NET SDK style project slowness in the editor #4889

Merged
merged 2 commits into from
May 14, 2018
Merged

Fixed .NET SDK style project slowness in the editor #4889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31b7f52
Removed updating project info on document open and document changed
TIHan May 12, 2018
faba1f0
Modified stress tests project gen to create netstandard projects
TIHan May 14, 2018
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
14 changes: 14 additions & 0 deletions tests/projects/stress/Templates/netstandard_fsproj.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
[FILES]
</ItemGroup>
<ItemGroup>
[PROJECTREFERENCES]
</ItemGroup>
<ItemGroup>
[PACKAGEREFERENCES]
</ItemGroup>
</Project>
23 changes: 18 additions & 5 deletions tests/projects/stress/gen.fsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,27 @@ open Perf
open System

printfn "%A" fsi.CommandLineArgs
if fsi.CommandLineArgs.Length <> 3 then printfn "usage: fsi gen.fsx <directory> <size>"
if fsi.CommandLineArgs.Length <> 4 then printfn "usage: fsi gen.fsx <directory> <size> <isNetStandard>"
let D = string fsi.CommandLineArgs.[1]
let N = int fsi.CommandLineArgs.[2]
let isNetStandard = Boolean.Parse(fsi.CommandLineArgs.[3])

try System.IO.Directory.Delete(D, true) with _ -> ()
System.IO.Directory.CreateDirectory D
System.Environment.CurrentDirectory <- D

let fsharpProjectWrite =
if isNetStandard then
FSharpProject.writeNetStandard
else
FSharpProject.write

let csharpProjectWrite =
if isNetStandard then
CSharpProject.writeNetStandard
else
CSharpProject.write

let writeDense (dir : string) (projectType : ProjectType) (count : int) =

let extension = match projectType with FSharp -> "fsproj" | CSharp -> "csproj"
Expand All @@ -30,7 +43,7 @@ let writeDense (dir : string) (projectType : ProjectType) (count : int) =
let fileName = sprintf "%s.fs" name
yield fileName ]
let project = { Name = name ; Guid = guid ; Files = files ; References = references ; BinaryReferences = [] }
let writer = match projectType with FSharp -> FSharpProject.write | CSharp -> CSharpProject.write
let writer = match projectType with FSharp -> fsharpProjectWrite | CSharp -> csharpProjectWrite
writer path project

projects |> List.iteri writeProject
Expand Down Expand Up @@ -62,7 +75,7 @@ let writeShallow (dir : string) (projectType : ProjectType) (count1 : int) (coun
let writeAProject (name, guid) =
let path = sprintf @"%s\%s\%s.%s" dir name name extension
let project = { Name = name ; Guid = guid ; Files = [] ; References = [] ; BinaryReferences = [] }
let writer = match projectType with FSharp -> FSharpProject.write | CSharp -> CSharpProject.write
let writer = match projectType with FSharp -> fsharpProjectWrite | CSharp -> csharpProjectWrite
writer path project

let writeBProject (name, guid) =
Expand All @@ -71,7 +84,7 @@ let writeShallow (dir : string) (projectType : ProjectType) (count1 : int) (coun
let makeRef (name, guid) = { Name = name ; Guid = guid ; RelativePath = sprintf @"..\%s\%s.%s" name name extension }
aProjects |> List.map makeRef
let project = { Name = name ; Guid = guid ; Files = [] ; References = references ; BinaryReferences = [] }
let writer = match projectType with FSharp -> FSharpProject.write | CSharp -> CSharpProject.write
let writer = match projectType with FSharp -> fsharpProjectWrite | CSharp -> csharpProjectWrite
writer path project

aProjects |> List.iter writeAProject
Expand Down Expand Up @@ -103,7 +116,7 @@ let writeDenseBin (dir : string) (projectType : ProjectType) (count : int) =
let makeRef (name, guid) : BinaryRef = { Name = name ; RelativePath = sprintf @"..\%s\bin\Debug\%s.dll" name name }
projects.[0..i-1] |> List.map makeRef
let project = { Name = name ; Guid = guid ; Files = [] ; References = [] ; BinaryReferences = references }
let writer = match projectType with FSharp -> FSharpProject.write | CSharp -> CSharpProject.write
let writer = match projectType with FSharp -> fsharpProjectWrite | CSharp -> csharpProjectWrite
writer path project

projects |> List.iteri writeProject
Expand Down
48 changes: 48 additions & 0 deletions tests/projects/stress/perf.fsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,30 @@ module FSharpProject =

safeWrite path text

let writeNetStandard path (project : Project) =

let files =
let printFileReference = sprintf " <Compile Include=\"%s\" />"
project.Files |> List.map printFileReference |> String.concat "\n"

for f in project.Files do
let filePath = Path.Combine(Path.GetDirectoryName(path),f)
let fileContents = sprintf "module %s\n\nlet x = 1" (Path.GetFileNameWithoutExtension(f))
safeWrite filePath fileContents

let references =
let printProjectReference (r : ProjectRef) =
sprintf " <ProjectReference Include=\"%s\" />" r.RelativePath
project.References |> List.map printProjectReference |> String.concat "\n"

let text =
File.ReadAllText(Path.Combine(__SOURCE_DIRECTORY__,@"Templates\netstandard_fsproj.template"))
.Replace("[FILES]", files)
.Replace("[PROJECTREFERENCES]", references)
.Replace("[PACKAGEREFERENCES]", String.Empty)

safeWrite path text

[<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
module CSharpProject =

Expand Down Expand Up @@ -135,6 +159,30 @@ module CSharpProject =

safeWrite path text

let writeNetStandard path (project : Project) =

let files =
let printFileReference = sprintf " <Compile Include=\"%s\" />"
project.Files |> List.map printFileReference |> String.concat "\n"

for f in project.Files do
let filePath = Path.Combine(Path.GetDirectoryName(path),f)
let fileContents = sprintf "module %s\n\nlet x = 1" (Path.GetFileNameWithoutExtension(f))
safeWrite filePath fileContents

let references =
let printProjectReference (r : ProjectRef) =
sprintf " <ProjectReference Include=\"%s\" />" r.RelativePath
project.References |> List.map printProjectReference |> String.concat "\n"

let text =
File.ReadAllText(Path.Combine(__SOURCE_DIRECTORY__,@"Templates\netstandard_fsproj.template"))
.Replace("[FILES]", files)
.Replace("[PROJECTREFERENCES]", references)
.Replace("[PACKAGEREFERENCES]", String.Empty)

safeWrite path text

module Testing =

let testWrite () =
Expand Down
14 changes: 1 addition & 13 deletions vsintegration/src/FSharp.Editor/LanguageService/LanguageService.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,6 @@ type internal FSharpLanguageService(package : FSharpPackage) =
member private this.OnProjectAdded(projectId:ProjectId) = projectInfoManager.UpdateProjectInfoWithProjectId(projectId, "OnProjectAdded", invalidateConfig=true)
member private this.OnProjectReloaded(projectId:ProjectId) = projectInfoManager.UpdateProjectInfoWithProjectId(projectId, "OnProjectReloaded", invalidateConfig=true)
member private this.OnDocumentAdded(projectId:ProjectId, documentId:DocumentId) = projectInfoManager.UpdateDocumentInfoWithProjectId(projectId, documentId, "OnDocumentAdded", invalidateConfig=true)
member private this.OnDocumentChanged(projectId:ProjectId, documentId:DocumentId) = projectInfoManager.UpdateDocumentInfoWithProjectId(projectId, documentId, "OnDocumentChanged", invalidateConfig=false)
member private this.OnDocumentReloaded(projectId:ProjectId, documentId:DocumentId) = projectInfoManager.UpdateDocumentInfoWithProjectId(projectId, documentId, "OnDocumentReloaded", invalidateConfig=true)

override this.Initialize() =
Expand All @@ -439,7 +438,6 @@ type internal FSharpLanguageService(package : FSharpPackage) =
| WorkspaceChangeKind.ProjectAdded -> this.OnProjectAdded(args.ProjectId)
| WorkspaceChangeKind.ProjectReloaded -> this.OnProjectReloaded(args.ProjectId)
| WorkspaceChangeKind.DocumentAdded -> this.OnDocumentAdded(args.ProjectId, args.DocumentId)
| WorkspaceChangeKind.DocumentChanged -> this.OnDocumentChanged(args.ProjectId, args.DocumentId)
Copy link
Member

Choose a reason for hiding this comment

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

Strange that DocumentChanged was in this list twice already. Also - consider whether DocumentInfoChanged should result in a call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably look into that. If we find that we need to add it in, we'll make a separate PR for that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking about this, once we move away from handlecommandlinargs, we will need to listen to DocumentInfoChanged most likely.

Copy link
Contributor

Choose a reason for hiding this comment

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

We didn't see any DocumentInfoChanged events - when do they arise?

Copy link
Member

Choose a reason for hiding this comment

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

For example when the filename changes, etc:

        /// <summary>
        /// The document in the current solution had is info changed; name, folders, filepath
        /// </summary>
        DocumentInfoChanged = 17,

| WorkspaceChangeKind.DocumentReloaded -> this.OnDocumentReloaded(args.ProjectId, args.DocumentId)
| WorkspaceChangeKind.DocumentRemoved
| WorkspaceChangeKind.ProjectRemoved
Expand Down Expand Up @@ -686,17 +684,7 @@ type internal FSharpLanguageService(package : FSharpPackage) =

let fileContents = VsTextLines.GetFileContents(textLines, textViewAdapter)
this.SetupStandAloneFile(filename, fileContents, this.Workspace, hier)
| id ->

// This is the path when opening in-project .fs/.fsi files in CPS projects when
// there is already an existing DocumentId for that document in the solution (which
// will normally be the case)
//
// However, it is not clear this call to UpdateProjectInfoWithProjectId is needed, and it seems
// harmful as it will cause a complete recheck of the project every time a view for a file in the
// project is freshly opened.

projectInfoManager.UpdateProjectInfoWithProjectId(id.ProjectId, "SetupNewTextView", invalidateConfig=true)
| _ -> ()
| _ ->

// This is the path for both in-project and out-of-project .fsx files
Expand Down