-
Notifications
You must be signed in to change notification settings - Fork 790
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
[WIP] Improve solution load performance #2909
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,12 @@ open Microsoft.VisualStudio | |
open Microsoft.VisualStudio.TextManager.Interop | ||
open Microsoft.VisualStudio.Shell.Interop | ||
open Microsoft.FSharp.Compiler.SourceCodeServices | ||
open System.Collections.Concurrent | ||
open System.Globalization | ||
|
||
type internal CacheUsage = | ||
| UseCache | ||
| IgnoreCache | ||
|
||
/// An additional interface that an IProjectSite object can implement to indicate it has an FSharpProjectOptions | ||
/// already available, so we don't have to recreate it | ||
|
@@ -121,26 +127,41 @@ type internal ProjectSitesAndFiles() = | |
| _ -> None) | ||
| None -> Seq.empty | ||
|
||
static let projectCache = ConcurrentDictionary<string,FSharpProjectOptions>() | ||
|
||
static let rec referencedProjectsOf (projectSite:IProjectSite, fileName, extraProjectInfo, serviceProvider:System.IServiceProvider) = | ||
referencedProvideProjectSites (projectSite, serviceProvider) | ||
|> Seq.choose (fun (p, ps) -> | ||
fullOutputAssemblyPath p | ||
|> Option.map (fun path -> | ||
path, getProjectOptionsForProjectSite (ps.GetProjectSite(), fileName, extraProjectInfo, serviceProvider)) | ||
path, getProjectOptionsForProjectSite (ps.GetProjectSite(), fileName, extraProjectInfo, serviceProvider, UseCache)) | ||
) | ||
|> Seq.toArray | ||
|
||
and getProjectOptionsForProjectSite(projectSite:IProjectSite, fileName, extraProjectInfo, serviceProvider) = | ||
{ProjectFileName = projectSite.ProjectFileName() | ||
ProjectFileNames = projectSite.SourceFilesOnDisk() | ||
OtherOptions = projectSite.CompilerFlags() | ||
ReferencedProjects = referencedProjectsOf(projectSite, fileName, extraProjectInfo, serviceProvider) | ||
IsIncompleteTypeCheckEnvironment = projectSite.IsIncompleteTypeCheckEnvironment | ||
UseScriptResolutionRules = SourceFile.MustBeSingleFileProject fileName | ||
LoadTime = projectSite.LoadTime | ||
UnresolvedReferences = None | ||
OriginalLoadReferences = [] | ||
ExtraProjectInfo=extraProjectInfo } | ||
and getProjectOptionsForProjectSite(projectSite:IProjectSite, fileName:string, extraProjectInfo, serviceProvider, cacheUsage:CacheUsage) = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will clash with my other proposed fix. The project options are already stored in a table in LanguageService.fs, it seems to me we should just callback to there to look up the table. |
||
let cacheKey = fileName.ToLower CultureInfo.CurrentUICulture | ||
match cacheUsage, projectCache.TryGetValue cacheKey with | ||
| UseCache, (true, options) -> | ||
{ options with ExtraProjectInfo = extraProjectInfo } | ||
| _ -> | ||
let options = | ||
{ | ||
ProjectFileName = projectSite.ProjectFileName() | ||
ProjectFileNames = projectSite.SourceFilesOnDisk() | ||
OtherOptions = projectSite.CompilerFlags() | ||
ReferencedProjects = referencedProjectsOf(projectSite, fileName, extraProjectInfo, serviceProvider) | ||
IsIncompleteTypeCheckEnvironment = projectSite.IsIncompleteTypeCheckEnvironment | ||
UseScriptResolutionRules = SourceFile.MustBeSingleFileProject fileName | ||
LoadTime = projectSite.LoadTime | ||
UnresolvedReferences = None | ||
OriginalLoadReferences = [] | ||
ExtraProjectInfo = extraProjectInfo | ||
} | ||
projectCache.[cacheKey] <- options | ||
options | ||
|
||
static member ClearCache() : unit = | ||
projectCache.Clear() | ||
|
||
/// Construct a project site for a single file. May be a single file project (for scripts) or an orphan project site (for everything else). | ||
static member ProjectSiteOfSingleFile(filename:string) : IProjectSite = | ||
|
@@ -227,9 +248,10 @@ type internal ProjectSitesAndFiles() = | |
/// Create project options for this project site. | ||
static member GetProjectOptionsForProjectSite(projectSite:IProjectSite,filename,extraProjectInfo,serviceProvider:System.IServiceProvider) = | ||
match projectSite with | ||
| :? IHaveCheckOptions as hco -> hco.OriginalCheckOptions() | ||
| :? IHaveCheckOptions as hco -> | ||
hco.OriginalCheckOptions() | ||
| _ -> | ||
getProjectOptionsForProjectSite(projectSite, filename, extraProjectInfo, serviceProvider) | ||
getProjectOptionsForProjectSite(projectSite, filename, extraProjectInfo, serviceProvider, IgnoreCache) | ||
|
||
/// Create project site for these project options | ||
static member CreateProjectSiteForScript (filename, checkOptions) = ProjectSiteOfScriptFile (filename, checkOptions) :> IProjectSite | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,7 +76,7 @@ public override string Url | |
{ | ||
get | ||
{ | ||
return this.myAssemblyPath; | ||
return Path.GetFullPath(this.myAssemblyPath); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fix still looks useful? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was used by the caching code to determine if two references were the same. |
||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -697,7 +697,7 @@ public override string Url | |
|
||
public void SetDebugLogger(Microsoft.Build.Framework.ILogger debugLogger) | ||
{ | ||
myDebugLogger = debugLogger; | ||
//myDebugLogger = debugLogger; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Temporary change, should not have committed |
||
} | ||
|
||
|
||
|
@@ -3128,7 +3128,9 @@ internal virtual void SetBuildConfigurationProperties(ConfigCanonicalName config | |
} | ||
} | ||
|
||
public abstract void NotifySourcesAndFlags(); | ||
public abstract void ComputeSourcesAndFlags(); | ||
public abstract IDisposable AcquireExclusiveBuildResource(bool actuallyBuild); | ||
|
||
internal abstract int FixupAppConfigOnTargetFXChange(string newTargetFramework, string targetFSharpCoreVersion, bool autoGenerateBindingRedirects); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -483,7 +483,7 @@ private string GetReferencedProjectOutputPath() | |
// build the full path adding the name of the assembly to the output path. | ||
outputPath = System.IO.Path.Combine(outputPath, assemblyName); | ||
|
||
return outputPath; | ||
return System.IO.Path.GetFullPath(outputPath); | ||
|
||
} | ||
|
||
|
@@ -872,11 +872,12 @@ private static System.Runtime.Versioning.FrameworkName GetProjectTargetFramework | |
if (hierarchy == null) | ||
return null; | ||
|
||
object otherTargetFrameworkMonikerObj; | ||
|
||
hierarchy.GetProperty(VSConstants.VSITEMID_ROOT, (int)__VSHPROPID4.VSHPROPID_TargetFrameworkMoniker, out otherTargetFrameworkMonikerObj); | ||
|
||
string targetFrameworkMoniker = (string)otherTargetFrameworkMonikerObj; | ||
object otargetFrameworkMoniker = null; | ||
int hr = hierarchy.GetProperty(VSConstants.VSITEMID_ROOT, (int)__VSHPROPID4.VSHPROPID_TargetFrameworkMoniker, out otargetFrameworkMoniker); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the correctness fix? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No this stops an occasional assertion on startup There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, thanks |
||
if (!ErrorHandler.Succeeded(hr)) | ||
return null; | ||
|
||
string targetFrameworkMoniker = (string)otargetFrameworkMoniker; | ||
return new System.Runtime.Versioning.FrameworkName(targetFrameworkMoniker); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -183,6 +183,8 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
type internal FSharpProjectPackage() as this = | ||
inherit ProjectPackage() | ||
|
||
let mutable flagsService = Unchecked.defaultof<ProjectFlagsService> | ||
|
||
let mutable vfsiToolWindow = Unchecked.defaultof<Microsoft.VisualStudio.FSharp.Interactive.FsiToolWindow> | ||
let GetToolWindowAsITestVFSI() = | ||
if vfsiToolWindow = Unchecked.defaultof<_> then | ||
|
@@ -199,6 +201,8 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
|
||
let thisLock = obj() | ||
|
||
member this.FlagsService = flagsService | ||
|
||
member this.RegisterForIdleTime() = | ||
|
||
if not (isNull mgr) && componentID = 0u then | ||
|
@@ -312,6 +316,13 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
//TODO the TypeProviderSecurityGlobals does not exists anymore, remove the initialization? | ||
this.GetService(typeof<FSharpLanguageService>) |> ignore | ||
|
||
//let flagsService = this.GetService(typeof<ProjectFlagsService>) :?> ProjectFlagsService | ||
//flagsService.Initialize() | ||
|
||
let accessor = this.GetService(typeof<SVsBuildManagerAccessor>) :?> IVsBuildManagerAccessor | ||
flagsService <- ProjectFlagsService(accessor) | ||
flagsService.Initialize() | ||
|
||
// FSI-LINKAGE-POINT: sited init | ||
let commandService = this.GetService(typeof<IMenuCommandService>) :?> OleMenuCommandService // FSI-LINKAGE-POINT | ||
Microsoft.VisualStudio.FSharp.Interactive.Hooks.fsiConsoleWindowPackageInitalizeSited (this :> Package) commandService | ||
|
@@ -473,7 +484,7 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
#if DEBUG | ||
let uiThreadId = System.Threading.Thread.CurrentThread.ManagedThreadId | ||
let mutable compileWasActuallyCalled = false | ||
#endif | ||
#endif | ||
|
||
// these get initialized once and for all in SetSite() | ||
let mutable isInCommandLineMode = false | ||
|
@@ -491,7 +502,7 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
this.ImageHandler.AddImage(FSharpSR.GetObject("4100") :?> System.Drawing.Bitmap) // 4008 = EmptyProject | ||
this.ImageHandler.AddImage(FSharpSR.GetObject("4103") :?> System.Drawing.Bitmap) // 4006 = ScriptFile | ||
this.ImageHandler.AddImage(FSharpSR.GetObject("4102") :?> System.Drawing.Bitmap) // 4007 = Signature | ||
|
||
/// Provide mapping from our browse objects and automation objects to our CATIDs | ||
do | ||
// The following properties classes are specific to F# so we can use their GUIDs directly | ||
|
@@ -1346,21 +1357,60 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
else | ||
0 | ||
|
||
// returns an array of all "foo"s of form: <Compile Include="foo"/> | ||
/// Returns an array of all files specified for compile in the build project. | ||
/// Returns all: <Compile Include="foo" /> | ||
member private x.ComputeCompileItems() = | ||
FSharpProjectNode.ComputeCompileItems(x.BuildProject, x.ProjectFolder) | ||
|
||
static member ComputeCompileItems(buildProject, projectFolder) = | ||
[| | ||
for i in buildProject.Items do | ||
if i.ItemType = "Compile" then | ||
yield System.IO.Path.GetFullPath(System.IO.Path.Combine(projectFolder, i.EvaluatedInclude)) | ||
|] | ||
member x.GetCompileItems() = let sources,_ = sourcesAndFlags.Value in sources | ||
member x.GetCompileFlags() = let _,flags = sourcesAndFlags.Value in flags | ||
|
||
/// Rudimentary approximation of compile flags | ||
member private x.ComputeCompileFlags() = | ||
[| | ||
for ref in x.GetReferenceContainer().EnumReferences() do | ||
match ref with | ||
| :? AssemblyReferenceNode -> | ||
yield sprintf "-r:%s" ref.Url | ||
| :? ProjectReferenceNode as projRef -> | ||
yield sprintf "-r:%s" projRef.ReferencedProjectOutputPath | ||
| _ -> () | ||
yield "--noframework" | ||
|] | ||
|
||
member x.GetCompileItems() = | ||
let sources, _ = sourcesAndFlags.Value in sources | ||
|
||
member x.GetCompileFlags() = | ||
let _, flags = sourcesAndFlags.Value in flags | ||
|
||
override this.AcquireExclusiveBuildResource(newActuallyBuild) = | ||
let oldActuallyBuild = actuallyBuild | ||
actuallyBuild <- newActuallyBuild | ||
|
||
{ new IDisposable with | ||
override __.Dispose() = | ||
actuallyBuild <- oldActuallyBuild | ||
} | ||
|
||
override this.NotifySourcesAndFlags() = | ||
sourcesAndFlagsNotifier.Notify() | ||
|
||
override this.ComputeSourcesAndFlags() = | ||
|
||
if not this.IsInBatchUpdate && box this.BuildProject <> null && not inMidstOfReloading then | ||
|
||
// TODO: estimate what our current flags/sources are | ||
|
||
override x.ComputeSourcesAndFlags() = | ||
//let flagsService : ProjectFlagsService = GetService(this.Site) | ||
package.FlagsService.QueueProject this | ||
|
||
if x.IsInBatchUpdate || box x.BuildProject = null then () | ||
(* | ||
if true || x.IsInBatchUpdate || box x.BuildProject = null then () | ||
else | ||
if not(inMidstOfReloading) && not(VsBuildManagerAccessorExtensionMethods.IsInProgress(accessor)) then | ||
|
||
|
@@ -1402,9 +1452,9 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
// taken from System.Runtime that is not supplied | ||
|
||
let _ = x.InvokeMsBuild("Compile", extraProperties = [KeyValuePair("_ResolveReferenceDependencies", "true")]) | ||
sourcesAndFlagsNotifier.Notify() | ||
finally | ||
actuallyBuild <- true | ||
*) | ||
|
||
member internal x.DetermineRuntimeAndSKU(targetFrameworkMoniker : string) = | ||
let frameworkName = new System.Runtime.Versioning.FrameworkName(targetFrameworkMoniker) | ||
|
@@ -1558,7 +1608,7 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
Debug.Assert(System.Threading.Thread.CurrentThread.ManagedThreadId = uiThreadId, "called GetProjectSite() while still in Opening state from non-UI thread") | ||
#endif | ||
x.ComputeSourcesAndFlags() | ||
if not(projectSite.State = ProjectSiteOptionLifetimeState.Opened) then | ||
if projectSite.State <> ProjectSiteOptionLifetimeState.Opened then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 :) |
||
// We failed to Build. This can happen e.g. when the user has custom MSBuild functionality under the "Compile" target, e.g. a CompileDependsOn that fails. | ||
// We now have no reliable way to get information about the project. | ||
// Rather than be in a completely useless state, we just report 0 source files and 0 compiler flags. | ||
|
@@ -1567,7 +1617,7 @@ namespace rec Microsoft.VisualStudio.FSharp.ProjectSystem | |
// Once that error is fixed, a future call to ComputeSourcesAndFlags() will successfully call through to our HostObject and get to Compile(), | ||
// which will finally populate sourcesAndFlags with good values. | ||
// This means that ones the user fixes the problem, proper intellisense etc. should start immediately lighting up. | ||
sourcesAndFlags <- Some([||],[||]) | ||
sourcesAndFlags <- Some(x.ComputeCompileItems(), [||]) // x.ComputeCompileFlags()) | ||
projectSite.Open(x.CreateRunningProjectSite()) | ||
() | ||
| _ -> () | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What removes entries from this cache?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing yet - good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed