Skip to content

Commit

Permalink
Fixed multi-targeted projects from invalidating the MruCache on docum…
Browse files Browse the repository at this point in the history
…ent open/changed (#4918)

* Handling multi-targeting in FCS by adding an optional ProjectId; handling multi-projects in a better way for LanguageService, making sure we clear it out of the options table

* Fixing tests with FSharpProjectOptions type changes

* Fixed more tests

* Fixed more tests

* Fixed more tests

* Fixed build
  • Loading branch information
TIHan authored May 16, 2018
1 parent d245100 commit a1f2f80
Show file tree
Hide file tree
Showing 24 changed files with 49 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ module Utils =

logMap := Map.add opts.ProjectFile opts.LogOutput !logMap
{ ProjectFileName = opts.ProjectFile
ProjectId = None
SourceFiles = sourceFiles
OtherOptions = otherOptions
ReferencedProjects = referencedProjects()
Expand Down
26 changes: 17 additions & 9 deletions src/fsharp/service/service.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1611,7 +1611,7 @@ module internal Parser =
userOpName: string) =

async {
use _logBlock = Logger.LogBlockMessage (Guid.NewGuid().ToString()) LogCompilerFunctionId.Service_CheckOneFile
use _logBlock = Logger.LogBlock LogCompilerFunctionId.Service_CheckOneFile

match parseResults.ParseTree with
// When processing the following cases, we don't need to type-check
Expand Down Expand Up @@ -1760,6 +1760,7 @@ type UnresolvedReferencesSet = UnresolvedReferencesSet of UnresolvedAssemblyRefe
type FSharpProjectOptions =
{
ProjectFileName: string
ProjectId: string option
SourceFiles: string[]
OtherOptions: string[]
ReferencedProjects: (string * FSharpProjectOptions)[]
Expand All @@ -1773,15 +1774,20 @@ type FSharpProjectOptions =
}
member x.ProjectOptions = x.OtherOptions
/// Whether the two parse options refer to the same project.
static member UseSameProjectFileName(options1,options2) =
options1.ProjectFileName = options2.ProjectFileName
static member UseSameProject(options1,options2) =
match options1.ProjectId, options2.ProjectId with
| Some(projectId1), Some(projectId2) when not (String.IsNullOrWhiteSpace(projectId1)) && not (String.IsNullOrWhiteSpace(projectId2)) ->
String.Equals(projectId1, projectId2, StringComparison.OrdinalIgnoreCase)
| Some(_), Some(_)
| None, None -> options1.ProjectFileName = options2.ProjectFileName
| _ -> false

/// Compare two options sets with respect to the parts of the options that are important to building.
static member AreSameForChecking(options1,options2) =
match options1.Stamp, options2.Stamp with
| Some x, Some y -> (x = y)
| _ ->
options1.ProjectFileName = options2.ProjectFileName &&
FSharpProjectOptions.UseSameProject(options1, options2) &&
options1.SourceFiles = options2.SourceFiles &&
options1.OtherOptions = options2.OtherOptions &&
options1.UnresolvedReferences = options2.UnresolvedReferences &&
Expand Down Expand Up @@ -2155,7 +2161,7 @@ module Helpers =
/// Determine whether two (fileName,options) keys should be identical w.r.t. resource usage
let AreSubsumable2((fileName1:string,o1:FSharpProjectOptions),(fileName2:string,o2:FSharpProjectOptions)) =
(fileName1 = fileName2)
&& FSharpProjectOptions.UseSameProjectFileName(o1,o2)
&& FSharpProjectOptions.UseSameProject(o1,o2)

/// Determine whether two (fileName,sourceText,options) keys should be identical w.r.t. parsing
let AreSameForParsing((fileName1: string, source1: string, options1), (fileName2, source2, options2)) =
Expand All @@ -2173,7 +2179,7 @@ module Helpers =
/// Determine whether two (fileName,sourceText,options) keys should be identical w.r.t. resource usage
let AreSubsumable3((fileName1:string,_,o1:FSharpProjectOptions),(fileName2:string,_,o2:FSharpProjectOptions)) =
(fileName1 = fileName2)
&& FSharpProjectOptions.UseSameProjectFileName(o1,o2)
&& FSharpProjectOptions.UseSameProject(o1,o2)

module CompileHelpers =
let mkCompilationErorHandlers() =
Expand Down Expand Up @@ -2316,7 +2322,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
let scriptClosureCache =
MruCache<ScriptClosureCacheToken, FSharpProjectOptions, LoadClosure>(projectCacheSize,
areSame=FSharpProjectOptions.AreSameForChecking,
areSimilar=FSharpProjectOptions.UseSameProjectFileName)
areSimilar=FSharpProjectOptions.UseSameProject)

let scriptClosureCacheLock = Lock<ScriptClosureCacheToken>()
let frameworkTcImportsCache = FrameworkImportsCache(frameworkTcImportsCacheStrongSize)
Expand Down Expand Up @@ -2389,7 +2395,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
MruCache<CompilationThreadToken, FSharpProjectOptions, (IncrementalBuilder option * FSharpErrorInfo[] * IDisposable)>
(keepStrongly=projectCacheSize, keepMax=projectCacheSize,
areSame = FSharpProjectOptions.AreSameForChecking,
areSimilar = FSharpProjectOptions.UseSameProjectFileName,
areSimilar = FSharpProjectOptions.UseSameProject,
requiredToKeep=(fun (builderOpt,_,_) -> match builderOpt with None -> false | Some (b:IncrementalBuilder) -> b.IsBeingKeptAliveApartFromCacheEntry),
onDiscard = (fun (_, _, decrement:IDisposable) -> decrement.Dispose()))

Expand Down Expand Up @@ -2660,7 +2666,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
let execWithReactorAsync action = reactor.EnqueueAndAwaitOpAsync(userOpName, "ParseAndCheckFileInProject", filename, action)
async {
try
let strGuid = "_" + Guid.NewGuid().ToString()
let strGuid = "_ProjectId=" + (options.ProjectId |> Option.defaultValue "null")
Logger.LogBlockMessageStart (filename + strGuid) LogCompilerFunctionId.Service_ParseAndCheckFileInProject

if implicitlyStartBackgroundWork then
Expand Down Expand Up @@ -2832,6 +2838,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
let options =
{
ProjectFileName = filename + ".fsproj" // Make a name that is unique in this directory.
ProjectId = None
SourceFiles = loadClosure.SourceFiles |> List.map fst |> List.toArray
OtherOptions = otherFlags
ReferencedProjects= [| |]
Expand Down Expand Up @@ -3184,6 +3191,7 @@ type FSharpChecker(legacyReferenceResolver, projectCacheSize, keepAssemblyConten
member ic.GetProjectOptionsFromCommandLineArgs(projectFileName, argv, ?loadedTimeStamp, ?extraProjectInfo: obj) =
let loadedTimeStamp = defaultArg loadedTimeStamp DateTime.MaxValue // Not 'now', we don't want to force reloading
{ ProjectFileName = projectFileName
ProjectId = None
SourceFiles = [| |] // the project file names will be inferred from the ProjectOptions
OtherOptions = argv
ReferencedProjects= [| |]
Expand Down
3 changes: 3 additions & 0 deletions src/fsharp/service/service.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,9 @@ type public FSharpProjectOptions =
// Note that this may not reduce to just the project directory, because there may be two projects in the same directory.
ProjectFileName: string

/// This is the unique identifier for the project, uses StringComparison.OrdinalIgnoreCase. If it's None, will key off of ProjectFileName in our caching.
ProjectId: string option

/// The files in the project
SourceFiles: string[]

Expand Down
1 change: 1 addition & 0 deletions tests/service/AssemblyContentProviderTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ let private filePath = "C:\\test.fs"

let private projectOptions : FSharpProjectOptions =
{ ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
1 change: 1 addition & 0 deletions tests/service/FileSystemTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ let ``FileSystem compilation test``() =
yield "-r:" + r |]

{ ProjectFileName = @"c:\mycode\compilation.fsproj" // Make a name that is unique in this directory.
ProjectId = None
SourceFiles = [| fileName1; fileName2 |]
OtherOptions = allFlags
ReferencedProjects = [| |];
Expand Down
4 changes: 4 additions & 0 deletions tests/service/MultiProjectAnalysisTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,7 @@ let ``Type provider project references should not throw exceptions`` () =
//let options = ProjectCracker.GetProjectOptionsFromProjectFile(projectFile, [("Configuration", "Debug")])
let options =
{ProjectFileName = __SOURCE_DIRECTORY__ + @"/data/TypeProviderConsole/TypeProviderConsole.fsproj";
ProjectId = None
SourceFiles = [|__SOURCE_DIRECTORY__ + @"/data/TypeProviderConsole/Program.fs"|];
Stamp = None
OtherOptions =
Expand All @@ -871,6 +872,7 @@ let ``Type provider project references should not throw exceptions`` () =
ReferencedProjects =
[|(__SOURCE_DIRECTORY__ + @"/data/TypeProviderLibrary/TypeProviderLibrary.dll",
{ProjectFileName = __SOURCE_DIRECTORY__ + @"/data/TypeProviderLibrary/TypeProviderLibrary.fsproj";
ProjectId = None
SourceFiles = [|__SOURCE_DIRECTORY__ + @"/data/TypeProviderLibrary/Library1.fs"|];
Stamp = None
OtherOptions =
Expand Down Expand Up @@ -935,6 +937,7 @@ let ``Projects creating generated types should not utilize cross-project-referen
let options =
{ProjectFileName =
__SOURCE_DIRECTORY__ + @"/data/TypeProvidersBug/TestConsole/TestConsole.fsproj";
ProjectId = None
SourceFiles =
[|__SOURCE_DIRECTORY__ + @"/data/TypeProvidersBug/TestConsole/AssemblyInfo.fs";
__SOURCE_DIRECTORY__ + @"/data/TypeProvidersBug/TestConsole/Program.fs"|];
Expand Down Expand Up @@ -963,6 +966,7 @@ let ``Projects creating generated types should not utilize cross-project-referen
[|(__SOURCE_DIRECTORY__ + @"/data/TypeProvidersBug/TypeProvidersBug/bin/Debug/TypeProvidersBug.dll",
{ProjectFileName =
__SOURCE_DIRECTORY__ + @"/data/TypeProvidersBug/TypeProvidersBug/TypeProvidersBug.fsproj";
ProjectId = None
SourceFiles =
[|__SOURCE_DIRECTORY__ + @"/data/TypeProvidersBug/TypeProvidersBug/AssemblyInfo.fs";
__SOURCE_DIRECTORY__ + @"/data/TypeProvidersBug/TypeProvidersBug/Library1.fs"|];
Expand Down
2 changes: 2 additions & 0 deletions vsintegration/Utils/LanguageServiceProfiling/Options.fs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ let FCS (repositoryDir: string) : Options =

{ Options =
{ProjectFileName = repositoryDir </> @"src\fsharp\FSharp.Compiler.Private\FSharp.Compiler.Private.fsproj"
ProjectId = None
SourceFiles = files |> Array.map (fun x -> repositoryDir </> x)
OtherOptions =
[|@"-o:obj\Release\FSharp.Compiler.Private.dll"; "-g"; "--noframework";
Expand Down Expand Up @@ -301,6 +302,7 @@ let FCS (repositoryDir: string) : Options =
let VFPT (repositoryDir: string) : Options =
{ Options =
{ProjectFileName = repositoryDir </> @"src\FSharp.Editing\FSharp.Editing.fsproj"
ProjectId = None
SourceFiles =
[|@"src\FSharp.Editing\AssemblyInfo.fs";
@"src\FSharp.Editing\Common\Utils.fs";
Expand Down
1 change: 0 additions & 1 deletion vsintegration/src/FSharp.Editor/Common/Extensions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ type System.IServiceProvider with
member x.GetService<'T>() = x.GetService(typeof<'T>) :?> 'T
member x.GetService<'S, 'T>() = x.GetService(typeof<'S>) :?> 'T


type FSharpNavigationDeclarationItem with
member x.RoslynGlyph : Glyph =
match x.Glyph with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,11 @@ type internal FSharpProjectOptionsManager

/// Update the info for a project in the project table
member this.UpdateProjectInfo(tryGetOrCreateProjectId, projectId, site, userOpName, invalidateConfig) =
Logger.LogMessage ("InvalidateConfig=" + invalidateConfig.ToString()) LogEditorFunctionId.LanguageService_UpdateProjectInfo
Logger.Log LogEditorFunctionId.LanguageService_UpdateProjectInfo
projectOptionsTable.AddOrUpdateProject(projectId, (fun isRefresh ->
let extraProjectInfo = Some(box workspace)
let tryGetOptionsForReferencedProject f = f |> tryGetOrCreateProjectId |> Option.bind this.TryGetOptionsForProject |> Option.map(fun (_, _, projectOptions) -> projectOptions)
let referencedProjects, projectOptions = ProjectSitesAndFiles.GetProjectOptionsForProjectSite(Settings.LanguageServicePerformance.EnableInMemoryCrossProjectReferences, tryGetOptionsForReferencedProject, site, serviceProvider, (tryGetOrCreateProjectId (site.ProjectFileName)), site.ProjectFileName, extraProjectInfo, Some projectOptionsTable)
let referencedProjects, projectOptions = ProjectSitesAndFiles.GetProjectOptionsForProjectSite(Settings.LanguageServicePerformance.EnableInMemoryCrossProjectReferences, tryGetOptionsForReferencedProject, site, serviceProvider, Some(projectId), site.ProjectFileName, extraProjectInfo, Some projectOptionsTable)
if invalidateConfig then checkerProvider.Checker.InvalidateConfiguration(projectOptions, startBackgroundCompileIfAlreadySeen = not isRefresh, userOpName = userOpName + ".UpdateProjectInfo")
let referencedProjectIds = referencedProjects |> Array.choose tryGetOrCreateProjectId
let parsingOptions, _ = checkerProvider.Checker.GetParsingOptionsFromProjectOptions(projectOptions)
Expand Down Expand Up @@ -427,6 +427,7 @@ 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.OnProjectRemoved(projectId) = projectInfoManager.ClearInfoForProject(projectId)
member private this.OnDocumentAdded(projectId:ProjectId, documentId:DocumentId) = projectInfoManager.UpdateDocumentInfoWithProjectId(projectId, documentId, "OnDocumentAdded", invalidateConfig=true)
member private this.OnDocumentReloaded(projectId:ProjectId, documentId:DocumentId) = projectInfoManager.UpdateDocumentInfoWithProjectId(projectId, documentId, "OnDocumentReloaded", invalidateConfig=true)

Expand All @@ -438,10 +439,10 @@ type internal FSharpLanguageService(package : FSharpPackage) =
match args.Kind with
| WorkspaceChangeKind.ProjectAdded -> this.OnProjectAdded(args.ProjectId)
| WorkspaceChangeKind.ProjectReloaded -> this.OnProjectReloaded(args.ProjectId)
| WorkspaceChangeKind.ProjectRemoved -> this.OnProjectRemoved(args.ProjectId)
| WorkspaceChangeKind.DocumentAdded -> this.OnDocumentAdded(args.ProjectId, args.DocumentId)
| WorkspaceChangeKind.DocumentReloaded -> this.OnDocumentReloaded(args.ProjectId, args.DocumentId)
| WorkspaceChangeKind.DocumentRemoved
| WorkspaceChangeKind.ProjectRemoved
| WorkspaceChangeKind.AdditionalDocumentAdded
| WorkspaceChangeKind.AdditionalDocumentReloaded
| WorkspaceChangeKind.AdditionalDocumentRemoved
Expand Down Expand Up @@ -694,4 +695,4 @@ type internal FSharpLanguageService(package : FSharpPackage) =
this.SetupStandAloneFile(filename, fileContents, this.Workspace, hier)

| _ -> ()
| _ -> ()
| _ -> ()
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ type internal ProjectSitesAndFiles() =
let option =
let newOption () = {
ProjectFileName = projectSite.ProjectFileName
ProjectId = projectId |> Option.map (fun x -> x.Id.ToString())
SourceFiles = projectSite.CompilationSourceFiles
OtherOptions = projectSite.CompilationOptions
ReferencedProjects = referencedProjectOptions
Expand Down
1 change: 1 addition & 0 deletions vsintegration/src/FSharp.LanguageService/FSharpSource.fs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ type internal FSharpSource_DEPRECATED(service:LanguageService_DEPRECATED, textLi
// get a sync parse of the file
let co, _ =
{ ProjectFileName = fileName + ".dummy.fsproj"
ProjectId = None
SourceFiles = [| fileName |]
OtherOptions = flags
ReferencedProjects = [| |]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ type internal ProjectSitesAndFiles() =
let option =
let newOption () = {
ProjectFileName = projectSite.ProjectFileName
ProjectId = None
SourceFiles = projectSite.CompilationSourceFiles
OtherOptions = projectSite.CompilationOptions
ReferencedProjects = referencedProjectOptions
Expand Down
1 change: 1 addition & 0 deletions vsintegration/tests/UnitTests/BraceMatchingServiceTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type BraceMatchingServiceTests() =
let fileName = "C:\\test.fs"
let projectOptions: FSharpProjectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| fileName |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type BreakpointResolutionServiceTests() =
let fileName = "C:\\test.fs"
let projectOptions: FSharpProjectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| fileName |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
1 change: 1 addition & 0 deletions vsintegration/tests/UnitTests/CompletionProviderTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ open UnitTests.TestLib.LanguageService
let filePath = "C:\\test.fs"
let internal projectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type DocumentDiagnosticAnalyzerTests() =
let endMarker = "(*end*)"
let projectOptions: FSharpProjectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ let filePath = "C:\\test.fs"

let internal projectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type EditorFormattingServiceTests() =
let filePath = "C:\\test.fs"
let projectOptions : FSharpProjectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ let _ = Module1.foo 1
let filePath = Path.GetTempFileName() + ".fs"
let options: FSharpProjectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
1 change: 1 addition & 0 deletions vsintegration/tests/UnitTests/HelpContextServiceTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type HelpContextServiceTests() =
let fileName = "C:\\test.fs"
let options: FSharpProjectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| fileName |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
1 change: 1 addition & 0 deletions vsintegration/tests/UnitTests/IndentationServiceTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type IndentationServiceTests() =
let filePath = "C:\\test.fs"
let projectOptions: FSharpProjectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
1 change: 1 addition & 0 deletions vsintegration/tests/UnitTests/QuickInfoProviderTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ let filePath = "C:\\test.fs"

let internal projectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ let PathRelativeToTestAssembly p = Path.Combine(Path.GetDirectoryName(Uri( Syste

let internal projectOptions = {
ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| "-r:" + PathRelativeToTestAssembly(@"UnitTests\MockTypeProviders\DummyProviderForLanguageServiceTesting.dll") |]
Expand Down
1 change: 1 addition & 0 deletions vsintegration/tests/UnitTests/UnusedOpensTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ let private filePath = "C:\\test.fs"

let private projectOptions : FSharpProjectOptions =
{ ProjectFileName = "C:\\test.fsproj"
ProjectId = None
SourceFiles = [| filePath |]
ReferencedProjects = [| |]
OtherOptions = [| |]
Expand Down

0 comments on commit a1f2f80

Please sign in to comment.