Skip to content

Commit

Permalink
Fix solution reload and config change issues (dotnet#3025)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@dsyme
dsyme authored May 16, 2017
1 parent 6bd50eb commit 6aced14
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 42 deletions.
2 changes: 1 addition & 1 deletion DocComments/XMLDocumentation.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<VsThreadToken,string,IVsXMLMemberIndex>(10,areSame=(fun (x,y) -> x = y))
let mutable xmlCache = new AgedLookup<VsThreadToken,string,IVsXMLMemberIndex>(10,areSimilar=(fun (x,y) -> x = y))

let events = dte.Events :?> Events2
let solutionEvents = events.SolutionEvents
Expand Down
127 changes: 91 additions & 36 deletions LanguageService/LanguageService.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
[<Export(typeof<ProjectInfoManager>); Composition.Shared>]
type internal ProjectInfoManager
Expand All @@ -73,30 +77,47 @@ type internal ProjectInfoManager
[<Import(typeof<SVsServiceProvider>)>] serviceProvider: System.IServiceProvider
) =
// A table of information about projects, excluding single-file projects.
let projectTable = ConcurrentDictionary<ProjectId, FSharpProjectOptions>()
let projectTable = ConcurrentDictionary<ProjectId, Refreshable<ProjectId[] * FSharpProjectOptions>>()

// 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<ProjectId, DateTime * FSharpProjectOptions>()

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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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<IWorkspaceProjectContext, string[]>()

override this.Initialize() =
base.Initialize()

Expand All @@ -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
Expand Down Expand Up @@ -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<string>) =
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) =
Expand All @@ -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<IWorkspaceProjectContextFactory>();
let errorReporter = ProjectExternalErrorReporter(projectId, "FS", this.SystemServiceProvider)

Expand All @@ -376,28 +435,24 @@ 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
project.AddProjectReference(ProjectReference referencedProjectId)
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<IWorkspaceProjectContextFactory>();
Expand Down
21 changes: 16 additions & 5 deletions LanguageService/Tokenizer.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -267,7 +268,7 @@ module internal Tokenizer =
data.[i] <- None
i <- i + 1

let private dataCache = ConditionalWeakTable<DocumentId, SourceTextData>()
let private dataCache = ConditionalWeakTable<DocumentId, ConcurrentDictionary<string list, SourceTextData>>()

let compilerTokenToRoslynToken(colorKind: FSharpTokenColorKind) : string =
match colorKind with
Expand Down Expand Up @@ -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<ClassifiedSpan> =
cancellationToken: CancellationToken) : List<ClassifiedSpan> =
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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 6aced14

Please sign in to comment.