From 6aced1475c4d77bd18894f700706cddc5237e436 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Tue, 16 May 2017 01:47:49 +0100 Subject: [PATCH] Fix solution reload and config change issues (#3025) * fix config change * Fix configuration changes * Fix colorization cache for changing defines * fix config switching * add manual test cases * Fix test * minor nit * use correct reference count * cleanup decrement logic * fix build * Update CompileOps.fs * adjust unit tests mocks * adjust unit tests mocks * fix test * add new misc project test * do ComputeSourcesAndFlags later * do ComputeSourcesAndFlags once * variations * keep dialog open --- DocComments/XMLDocumentation.fs | 2 +- LanguageService/LanguageService.fs | 127 +++++++++++++++++++++-------- LanguageService/Tokenizer.fs | 21 +++-- 3 files changed, 108 insertions(+), 42 deletions(-) diff --git a/DocComments/XMLDocumentation.fs b/DocComments/XMLDocumentation.fs index 072729234b2..717466c380e 100644 --- a/DocComments/XMLDocumentation.fs +++ b/DocComments/XMLDocumentation.fs @@ -214,7 +214,7 @@ module internal XmlDocumentation = /// Provide Xml Documentation type Provider(xmlIndexService:IVsXMLMemberIndexService, dte: DTE) = /// Index of assembly name to xml member index. - let mutable xmlCache = new AgedLookup(10,areSame=(fun (x,y) -> x = y)) + let mutable xmlCache = new AgedLookup(10,areSimilar=(fun (x,y) -> x = y)) let events = dte.Events :?> Events2 let solutionEvents = events.SolutionEvents diff --git a/LanguageService/LanguageService.fs b/LanguageService/LanguageService.fs index 6bc55e08799..666e288e11e 100644 --- a/LanguageService/LanguageService.fs +++ b/LanguageService/LanguageService.fs @@ -8,6 +8,7 @@ open System open System.Collections.Concurrent open System.Collections.Generic open System.ComponentModel.Composition +open System.Runtime.CompilerServices open System.Runtime.InteropServices open System.IO open System.Diagnostics @@ -64,6 +65,9 @@ type internal FSharpCheckerProvider member this.Checker = checker.Value +/// A value and a function to recompute/refresh the value. The function is passed a flag indicating if a refresh is happening. +type Refreshable<'T> = 'T * (bool -> 'T) + // Exposes project information as MEF component [); Composition.Shared>] type internal ProjectInfoManager @@ -73,30 +77,47 @@ type internal ProjectInfoManager [)>] serviceProvider: System.IServiceProvider ) = // A table of information about projects, excluding single-file projects. - let projectTable = ConcurrentDictionary() + let projectTable = ConcurrentDictionary>() // A table of information about single-file projects. Currently we only need the load time of each such file, plus // the original options for editing let singleFileProjectTable = ConcurrentDictionary() - member this.AddSingleFileProject(projectId, timeStampAndOptions) = - singleFileProjectTable.TryAdd(projectId, timeStampAndOptions) |> ignore + /// Clear a project from the project table + member this.ClearInfoForProject(projectId: ProjectId) = + projectTable.TryRemove(projectId) |> ignore + this.RefreshInfoForProjectsThatReferenceThisProject(projectId) - member this.RemoveSingleFileProject(projectId) = + member this.ClearInfoForSingleFileProject(projectId) = singleFileProjectTable.TryRemove(projectId) |> ignore - /// Clear a project from the project table - member this.ClearProjectInfo(projectId: ProjectId) = - projectTable.TryRemove(projectId) |> ignore + member this.RefreshInfoForProjectsThatReferenceThisProject(projectId: ProjectId) = + // Search the projectTable for things to refresh + for KeyValue(otherProjectId, ((referencedProjectIds, _options), refresh)) in projectTable.ToArray() do + for referencedProjectId in referencedProjectIds do + if referencedProjectId = projectId then + projectTable.[otherProjectId] <- (refresh true, refresh) + + member this.AddOrUpdateProject(projectId, refresh) = + projectTable.[projectId] <- (refresh false, refresh) + this.RefreshInfoForProjectsThatReferenceThisProject(projectId) + + member this.AddOrUpdateSingleFileProject(projectId, data) = + singleFileProjectTable.[projectId] <- data /// Get the exact options for a single-file script member this.ComputeSingleFileOptions (tryGetOrCreateProjectId, fileName, loadTime, fileContents, workspace: Workspace) = async { let extraProjectInfo = Some(box workspace) let tryGetOptionsForReferencedProject f = f |> tryGetOrCreateProjectId |> Option.bind this.TryGetOptionsForProject if SourceFile.MustBeSingleFileProject(fileName) then - let optionsStamp = None // TODO: can we use a unique stamp? + // NOTE: we don't use a unique stamp for single files, instead comparing options structurally. + // This is because we repeatedly recompute the options. + let optionsStamp = None let! options, _diagnostics = checkerProvider.Checker.GetProjectOptionsFromScript(fileName, fileContents, loadTime, [| |], ?extraProjectInfo=extraProjectInfo, ?optionsStamp=optionsStamp) - let site = ProjectSitesAndFiles.CreateProjectSiteForScript(fileName, options) + // NOTE: we don't use FCS cross-project references from scripts to projects. THe projects must have been + // compiled and #r will refer to files on disk + let referencedProjectFileNames = [| |] + let site = ProjectSitesAndFiles.CreateProjectSiteForScript(fileName, referencedProjectFileNames, options) return ProjectSitesAndFiles.GetProjectOptionsForProjectSite(tryGetOptionsForReferencedProject,site,fileName,options.ExtraProjectInfo,serviceProvider, true) else let site = ProjectSitesAndFiles.ProjectSiteOfSingleFile(fileName) @@ -105,11 +126,13 @@ type internal ProjectInfoManager /// Update the info for a project in the project table member this.UpdateProjectInfo(tryGetOrCreateProjectId, projectId: ProjectId, site: IProjectSite, workspace: Workspace) = - let extraProjectInfo = Some(box workspace) - let tryGetOptionsForReferencedProject f = f |> tryGetOrCreateProjectId |> Option.bind this.TryGetOptionsForProject - let options = ProjectSitesAndFiles.GetProjectOptionsForProjectSite(tryGetOptionsForReferencedProject, site, site.ProjectFileName(), extraProjectInfo, serviceProvider, true) - checkerProvider.Checker.InvalidateConfiguration(options) - projectTable.[projectId] <- options + this.AddOrUpdateProject(projectId, (fun isRefresh -> + let extraProjectInfo = Some(box workspace) + let tryGetOptionsForReferencedProject f = f |> tryGetOrCreateProjectId |> Option.bind this.TryGetOptionsForProject + let referencedProjects, options = ProjectSitesAndFiles.GetProjectOptionsForProjectSite(tryGetOptionsForReferencedProject, site, site.ProjectFileName(), extraProjectInfo, serviceProvider, true) + let referencedProjectIds = referencedProjects |> Array.choose tryGetOrCreateProjectId + checkerProvider.Checker.InvalidateConfiguration(options, startBackgroundCompile = not isRefresh) + referencedProjectIds, options)) /// Get compilation defines relevant for syntax processing. /// Quicker then TryGetOptionsForDocumentOrProject as it doesn't need to recompute the exact project @@ -125,7 +148,7 @@ type internal ProjectInfoManager /// Get the options for a project member this.TryGetOptionsForProject(projectId: ProjectId) = match projectTable.TryGetValue(projectId) with - | true, options -> Some options + | true, ((_referencedProjects, options), _) -> Some options | _ -> None /// Get the exact options for a document or project @@ -136,13 +159,16 @@ type internal ProjectInfoManager // single-file project may contain #load and #r references which are changing as the user edits, and we may need to re-analyze // to determine the latest settings. FCS keeps a cache to help ensure these are up-to-date. match singleFileProjectTable.TryGetValue(projectId) with - | true, (loadTime,_) -> + | true, (loadTime, _) -> try let fileName = document.FilePath let! cancellationToken = Async.CancellationToken let! sourceText = document.GetTextAsync(cancellationToken) - let! options = this.ComputeSingleFileOptions ((fun _ -> None), fileName, loadTime, sourceText.ToString(), document.Project.Solution.Workspace) - singleFileProjectTable.[projectId] <- (loadTime, options) + // NOTE: we don't use FCS cross-project references from scripts to projects. The projects must have been + // compiled and #r will refer to files on disk. + let tryGetOrCreateProjectId _ = None + let! _referencedProjectFileNames, options = this.ComputeSingleFileOptions (tryGetOrCreateProjectId, fileName, loadTime, sourceText.ToString(), document.Project.Solution.Workspace) + this.AddOrUpdateSingleFileProject(projectId, (loadTime, options)) return Some options with ex -> Assert.Exception(ex) @@ -261,13 +287,19 @@ and let tryRemoveSingleFileProject projectId = match singleFileProjects.TryRemove(projectId) with | true, project -> - projectInfoManager.RemoveSingleFileProject(projectId) + projectInfoManager.ClearInfoForSingleFileProject(projectId) project.Disconnect() | _ -> () let invalidPathChars = set (Path.GetInvalidPathChars()) let isPathWellFormed (path: string) = not (String.IsNullOrWhiteSpace path) && path |> Seq.forall (fun c -> not (Set.contains c invalidPathChars)) + let tryGetOrCreateProjectId (workspace: VisualStudioWorkspaceImpl) (projectFileName: string) = + let projectDisplayName = projectDisplayNameOf projectFileName + Some (workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)) + + let optionsAssociation = ConditionalWeakTable() + override this.Initialize() = base.Initialize() @@ -278,6 +310,14 @@ and tryRemoveSingleFileProject args.Document.Project.Id Events.SolutionEvents.OnAfterCloseSolution.Add <| fun _ -> + //checkerProvider.Checker.StopBackgroundCompile() + + // FUTURE: consider enbling some or all of these to flush all caches and stop all background builds. However the operations + // are asynchronous and we need to decide if we stop everything synchronously. + + //checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients() + //checkerProvider.Checker.InvalidateAll() + singleFileProjects.Keys |> Seq.iter tryRemoveSingleFileProject let ctx = System.Threading.SynchronizationContext.Current @@ -309,39 +349,58 @@ and /// Sync the information for the project member this.SyncProject(project: AbstractProject, projectContext: IWorkspaceProjectContext, site: IProjectSite, workspace, forceUpdate) = let wellFormedFilePathSetIgnoreCase (paths: seq) = - HashSet(paths |> Seq.filter isPathWellFormed, StringComparer.OrdinalIgnoreCase) + HashSet(paths |> Seq.filter isPathWellFormed |> Seq.map (fun s -> try System.IO.Path.GetFullPath(s) with _ -> s), StringComparer.OrdinalIgnoreCase) let updatedFiles = site.SourceFilesOnDisk() |> wellFormedFilePathSetIgnoreCase - let workspaceFiles = project.GetCurrentDocuments() |> Seq.map (fun file -> file.FilePath) |> wellFormedFilePathSetIgnoreCase + let originalFiles = project.GetCurrentDocuments() |> Seq.map (fun file -> file.FilePath) |> wellFormedFilePathSetIgnoreCase let mutable updated = forceUpdate for file in updatedFiles do - if not(workspaceFiles.Contains(file)) then + if not(originalFiles.Contains(file)) then projectContext.AddSourceFile(file) updated <- true - for file in workspaceFiles do + for file in originalFiles do if not(updatedFiles.Contains(file)) then projectContext.RemoveSourceFile(file) updated <- true let updatedRefs = site.AssemblyReferences() |> wellFormedFilePathSetIgnoreCase - let workspaceRefs = project.GetCurrentMetadataReferences() |> Seq.map (fun ref -> ref.FilePath) |> wellFormedFilePathSetIgnoreCase + let originalRefs = project.GetCurrentMetadataReferences() |> Seq.map (fun ref -> ref.FilePath) |> wellFormedFilePathSetIgnoreCase for ref in updatedRefs do - if not(workspaceRefs.Contains(ref)) then + if not(originalRefs.Contains(ref)) then projectContext.AddMetadataReference(ref, MetadataReferenceProperties.Assembly) updated <- true - for ref in workspaceRefs do + for ref in originalRefs do if not(updatedRefs.Contains(ref)) then projectContext.RemoveMetadataReference(ref) updated <- true + let ok,originalOptions = optionsAssociation.TryGetValue(projectContext) + let updatedOptions = site.CompilerFlags() + if not ok || originalOptions <> updatedOptions then + + // OK, project options have changed, try to fake out Roslyn to convince it to reparse things. + // Calling SetOptions fails because the CPS project system being used by the F# project system + // imlpementation at the moment has no command line parser installed, so we remove/add all the files + // instead. A change of flags doesn't happen very often and the remove/add is fast in any case. + //projectContext.SetOptions(String.concat " " updatedOptions) + for file in updatedFiles do + projectContext.RemoveSourceFile(file) + projectContext.AddSourceFile(file) + + // Record the last seen options as an associated value + if ok then optionsAssociation.Remove(projectContext) |> ignore + optionsAssociation.Add(projectContext, updatedOptions) + + updated <- true + // update the cached options if updated then - projectInfoManager.UpdateProjectInfo(this.GetProjectIdForReferencedProject workspace, project.Id, site, project.Workspace) + projectInfoManager.UpdateProjectInfo(tryGetOrCreateProjectId workspace, project.Id, site, project.Workspace) member this.SetupProjectFile(siteProvider: IProvideProjectSite, workspace: VisualStudioWorkspaceImpl) = let rec setup (site: IProjectSite) = @@ -351,7 +410,7 @@ and let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName) if isNull (workspace.ProjectTracker.GetProject projectId) then - projectInfoManager.UpdateProjectInfo(this.GetProjectIdForReferencedProject workspace, projectId, site, workspace) + projectInfoManager.UpdateProjectInfo(tryGetOrCreateProjectId workspace, projectId, site, workspace) let projectContextFactory = package.ComponentModel.GetService(); let errorReporter = ProjectExternalErrorReporter(projectId, "FS", this.SystemServiceProvider) @@ -376,7 +435,8 @@ and AdviseProjectSiteChanges(fun () -> this.SyncProject(project, projectContext, site, workspace, forceUpdate=true))) site.AdviseProjectSiteClosed(FSharpConstants.FSharpLanguageServiceCallbackName, AdviseProjectSiteChanges(fun () -> - projectInfoManager.ClearProjectInfo(project.Id) + projectInfoManager.ClearInfoForProject(project.Id) + optionsAssociation.Remove(projectContext) |> ignore project.Disconnect())) for referencedSite in ProjectSitesAndFiles.GetReferencedProjectSites (site, this.SystemServiceProvider) do let referencedProjectId = setup referencedSite @@ -384,20 +444,15 @@ and projectId setup (siteProvider.GetProjectSite()) |> ignore - member this.GetProjectIdForReferencedProject (workspace: VisualStudioWorkspaceImpl) (projectFileName: string) = - let projectDisplayName = projectDisplayNameOf projectFileName - Some (workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName)) - member this.SetupStandAloneFile(fileName: string, fileContents: string, workspace: VisualStudioWorkspaceImpl, hier: IVsHierarchy) = let loadTime = DateTime.Now - let options = projectInfoManager.ComputeSingleFileOptions (this.GetProjectIdForReferencedProject workspace, fileName, loadTime, fileContents, workspace) |> Async.RunSynchronously - let projectFileName = fileName let projectDisplayName = projectDisplayNameOf projectFileName let projectId = workspace.ProjectTracker.GetOrCreateProjectIdForPath(projectFileName, projectDisplayName) - projectInfoManager.AddSingleFileProject(projectId, (loadTime, options)) + let _referencedProjectFileNames, options = projectInfoManager.ComputeSingleFileOptions (tryGetOrCreateProjectId workspace, fileName, loadTime, fileContents, workspace) |> Async.RunSynchronously + projectInfoManager.AddOrUpdateSingleFileProject(projectId, (loadTime, options)) if isNull (workspace.ProjectTracker.GetProject projectId) then let projectContextFactory = package.ComponentModel.GetService(); diff --git a/LanguageService/Tokenizer.fs b/LanguageService/Tokenizer.fs index 01e52c13b80..f811f783888 100644 --- a/LanguageService/Tokenizer.fs +++ b/LanguageService/Tokenizer.fs @@ -2,6 +2,7 @@ open System open System.Collections.Generic +open System.Collections.Concurrent open System.Threading open System.Threading.Tasks open System.Runtime.CompilerServices @@ -267,7 +268,7 @@ module internal Tokenizer = data.[i] <- None i <- i + 1 - let private dataCache = ConditionalWeakTable() + let private dataCache = ConditionalWeakTable>() let compilerTokenToRoslynToken(colorKind: FSharpTokenColorKind) : string = match colorKind with @@ -346,13 +347,23 @@ module internal Tokenizer = SourceLineData(textLine.Start, lexState, previousLexState.Value, lineContents.GetHashCode(), classifiedSpans, List.ofSeq tokens) + + // We keep incremental data per-document. When text changes we correlate text line-by-line (by hash codes of lines) + // We index the data by the active defines in the document. + let private getSourceTextData(documentKey: DocumentId, defines: string list, linesCount) = + let dict = dataCache.GetValue(documentKey, fun key -> new ConcurrentDictionary<_,_>(1,1,HashIdentity.Structural)) + if dict.ContainsKey(defines) then dict.[defines] + else + let data = SourceTextData(linesCount) + dict.TryAdd(defines, data) |> ignore + data + let getColorizationData(documentKey: DocumentId, sourceText: SourceText, textSpan: TextSpan, fileName: string option, defines: string list, - cancellationToken: CancellationToken) : List = + cancellationToken: CancellationToken) : List = try let sourceTokenizer = FSharpSourceTokenizer(defines, fileName) let lines = sourceText.Lines - // We keep incremental data per-document. When text changes we correlate text line-by-line (by hash codes of lines) - let sourceTextData = dataCache.GetValue(documentKey, fun key -> SourceTextData(lines.Count)) + let sourceTextData = getSourceTextData(documentKey, defines, lines.Count) let startLine = lines.GetLineFromPosition(textSpan.Start).LineNumber let endLine = lines.GetLineFromPosition(textSpan.End).LineNumber @@ -539,7 +550,7 @@ module internal Tokenizer = let sourceTokenizer = FSharpSourceTokenizer(defines, Some fileName) let lines = sourceText.Lines // We keep incremental data per-document. When text changes we correlate text line-by-line (by hash codes of lines) - let sourceTextData = dataCache.GetValue(documentKey, fun key -> SourceTextData(lines.Count)) + let sourceTextData = getSourceTextData(documentKey, defines, lines.Count) // Go backwards to find the last cached scanned line that is valid let scanStartLine = let mutable i = min (lines.Count - 1) lineNumber