Refactoring of IncrementalBuilder (#14903)

* one pass mapFold, cache parse tasks

* refactor a bit

* ah, ok

* works again?

* some refactoring

* fix test for now

* lock it

* better

* cleanup

* more correct?

* not in use

* restore test

* wip

* refactor away partialCheck flag where possible

* fantomas

* fix

* ok

* add this back

* format

* fmt

* wip

* restore enablePartialTypeChecking

* restore enablePartialTypeChecking

* fix conflict

* naming stuff

* fix test

* task run

* update docs

* use SemaphoreSlim

* add a comment

* fix

* revert to lock

* make full type check more parallel

* Revert "make full type check more parallel"

This reverts commit 632b3ba430e0e933c75de71518f36f2e97d22a35.

* refactor BoundModel, parallel typecheck when possible

* use GraphNode to keep parsing result

* fantomas

* commited too much, fix

* fix bug, fix tests

* eager parsing

* not useful and not deterministic

* remove eager parsing

* oh, well

* cleanup

* do not compute temp states

* remove private

* reclaim type check memory faster

* dont check when state is given

* less ugly

---------
Co-authored-by: NTomas Grosup <tomasgrosup@microsoft.com>

src/Compiler/Driver/ParseAndCheckInputs.fs

@@ -1156,10 +1156,6 @@ let GetInitialTcState (m, ccuName, tcConfig: TcConfig, tcGlobals, tcImports: TcI
         tcsImplicitOpenDeclarations = openDecls0
     }
 
-/// Dummy typed impl file that contains no definitions and is not used for emitting any kind of assembly.
-let CreateEmptyDummyImplFile qualNameOfFile sigTy =
-    CheckedImplFile(qualNameOfFile, [], sigTy, ModuleOrNamespaceContents.TMDefs [], false, false, StampMap [], Map.empty)
-
 let AddCheckResultsToTcState
     (tcGlobals, amap, hadSig, prefixPathOpt, tcSink, tcImplEnv, qualNameOfFile, implFileSigType)
     (tcState: TcState)
@@ -1208,28 +1204,68 @@ let AddCheckResultsToTcState
 
 type PartialResult = TcEnv * TopAttribs * CheckedImplFile option * ModuleOrNamespaceType
 
-/// Typecheck a single file (or interactive entry into F# Interactive)
-let CheckOneInputAux
+/// Returns partial type check result for skipped implementation files.
+let SkippedImplFilePlaceholder (tcConfig: TcConfig, tcImports: TcImports, tcGlobals, tcState, input: ParsedInput) =
+    use _ =
+        Activity.start "ParseAndCheckInputs.SkippedImplFilePlaceholder" [| Activity.Tags.fileName, input.FileName |]
+
+    CheckSimulateException tcConfig
+
+    match input with
+    | ParsedInput.ImplFile file ->
+        let qualNameOfFile = file.QualifiedName
+
+        // Check if we've got an interface for this fragment
+        let rootSigOpt = tcState.tcsRootSigs.TryFind qualNameOfFile
+
+        // Check if we've already seen an implementation for this fragment
+        if Zset.contains qualNameOfFile tcState.tcsRootImpls then
+            errorR (Error(FSComp.SR.buildImplementationAlreadyGiven (qualNameOfFile.Text), input.Range))
+
+        let hadSig = rootSigOpt.IsSome
+
+        match rootSigOpt with
+        | Some rootSigTy ->
+            // Delay the typecheck the implementation file until the second phase of parallel processing.
+            // Adjust the TcState as if it has been checked, which makes the signature for the file available later
+            // in the compilation order.
+            let tcStateForImplFile = tcState
+            let amap = tcImports.GetImportMap()
+
+            let ccuSigForFile, tcState =
+                AddCheckResultsToTcState
+                    (tcGlobals, amap, hadSig, None, TcResultsSink.NoSink, tcState.tcsTcImplEnv, qualNameOfFile, rootSigTy)
+                    tcState
+
+            let emptyImplFile =
+                CheckedImplFile(qualNameOfFile, [], rootSigTy, ModuleOrNamespaceContents.TMDefs [], false, false, StampMap [], Map.empty)
+
+            let tcEnvAtEnd = tcStateForImplFile.TcEnvFromImpls
+            Some((tcEnvAtEnd, EmptyTopAttrs, Some emptyImplFile, ccuSigForFile), tcState)
+
+        | _ -> None
+    | _ -> None
+
+/// Typecheck a single file (or interactive entry into F# Interactive).
+let CheckOneInput
     (
         checkForErrors,
         tcConfig: TcConfig,
         tcImports: TcImports,
-        tcGlobals,
-        prefixPathOpt,
-        tcSink,
+        tcGlobals: TcGlobals,
+        prefixPathOpt: LongIdent option,
+        tcSink: TcResultsSink,
         tcState: TcState,
-        inp: ParsedInput,
-        skipImplIfSigExists: bool
-    ) =
-
+        input: ParsedInput
+    ) : Cancellable<PartialResult * TcState> =
     cancellable {
         try
             use _ =
-                Activity.start "ParseAndCheckInputs.CheckOneInput" [| Activity.Tags.fileName, inp.FileName |]
+                Activity.start "ParseAndCheckInputs.CheckOneInput" [| Activity.Tags.fileName, input.FileName |]
 
             CheckSimulateException tcConfig
 
-            let m = inp.Range
+            let m = input.Range
             let amap = tcImports.GetImportMap()
 
             let conditionalDefines =
@@ -1238,7 +1274,7 @@ let CheckOneInputAux
                 else
                     Some tcConfig.conditionalDefines
 
-            match inp with
+            match input with
             | ParsedInput.SigFile file ->
                 let qualNameOfFile = file.QualifiedName
 
@@ -1285,7 +1321,7 @@ let CheckOneInputAux
                         tcsCreatesGeneratedProvidedTypes = tcState.tcsCreatesGeneratedProvidedTypes || createsGeneratedProvidedTypes
                     }
 
-                return Choice1Of2(tcEnv, EmptyTopAttrs, None, ccuSigForFile), tcState
+                return (tcEnv, EmptyTopAttrs, None, ccuSigForFile), tcState
 
             | ParsedInput.ImplFile file ->
                 let qualNameOfFile = file.QualifiedName
@@ -1299,84 +1335,39 @@ let CheckOneInputAux
 
                 let hadSig = rootSigOpt.IsSome
 
-                match rootSigOpt with
-                | Some rootSig when skipImplIfSigExists ->
-                    // Delay the typecheck the implementation file until the second phase of parallel processing.
-                    // Adjust the TcState as if it has been checked, which makes the signature for the file available later
-                    // in the compilation order.
-                    let tcStateForImplFile = tcState
-                    let qualNameOfFile = file.QualifiedName
-                    let priorErrors = checkForErrors ()
-
-                    let ccuSigForFile, tcState =
-                        AddCheckResultsToTcState
-                            (tcGlobals, amap, hadSig, prefixPathOpt, tcSink, tcState.tcsTcImplEnv, qualNameOfFile, rootSig)
-                            tcState
+                // Typecheck the implementation file
+                let! topAttrs, implFile, tcEnvAtEnd, createsGeneratedProvidedTypes =
+                    CheckOneImplFile(
+                        tcGlobals,
+                        amap,
+                        tcState.tcsCcu,
+                        tcState.tcsImplicitOpenDeclarations,
+                        checkForErrors,
+                        conditionalDefines,
+                        tcSink,
+                        tcConfig.internalTestSpanStackReferring,
+                        tcState.tcsTcImplEnv,
+                        rootSigOpt,
+                        file,
+                        tcConfig.diagnosticsOptions
+                    )
 
-                    let partialResult =
-                        (amap, conditionalDefines, rootSig, priorErrors, file, tcStateForImplFile, ccuSigForFile)
+                let tcState =
+                    { tcState with
+                        tcsCreatesGeneratedProvidedTypes = tcState.tcsCreatesGeneratedProvidedTypes || createsGeneratedProvidedTypes
+                    }
 
-                    return Choice2Of2 partialResult, tcState
+                let ccuSigForFile, tcState =
+                    AddCheckResultsToTcState
+                        (tcGlobals, amap, hadSig, prefixPathOpt, tcSink, tcState.tcsTcImplEnv, qualNameOfFile, implFile.Signature)
+                        tcState
 
-                | _ ->
-                    // Typecheck the implementation file
-                    let! topAttrs, implFile, tcEnvAtEnd, createsGeneratedProvidedTypes =
-                        CheckOneImplFile(
-                            tcGlobals,
-                            amap,
-                            tcState.tcsCcu,
-                            tcState.tcsImplicitOpenDeclarations,
-                            checkForErrors,
-                            conditionalDefines,
-                            tcSink,
-                            tcConfig.internalTestSpanStackReferring,
-                            tcState.tcsTcImplEnv,
-                            rootSigOpt,
-                            file,
-                            tcConfig.diagnosticsOptions
-                        )
-
-                    let tcState =
-                        { tcState with
-                            tcsCreatesGeneratedProvidedTypes = tcState.tcsCreatesGeneratedProvidedTypes || createsGeneratedProvidedTypes
-                        }
-
-                    let ccuSigForFile, tcState =
-                        AddCheckResultsToTcState
-                            (tcGlobals, amap, hadSig, prefixPathOpt, tcSink, tcState.tcsTcImplEnv, qualNameOfFile, implFile.Signature)
-                            tcState
-
-                    let result = (tcEnvAtEnd, topAttrs, Some implFile, ccuSigForFile)
-                    return Choice1Of2 result, tcState
+                let result = (tcEnvAtEnd, topAttrs, Some implFile, ccuSigForFile)
+                return result, tcState
 
         with e ->
             errorRecovery e range0
-            return Choice1Of2(tcState.TcEnvFromSignatures, EmptyTopAttrs, None, tcState.tcsCcuSig), tcState
-    }
-
-/// Typecheck a single file (or interactive entry into F# Interactive). If skipImplIfSigExists is set to true
-/// then implementations with signature files give empty results.
-let CheckOneInput
-    ((checkForErrors,
-      tcConfig: TcConfig,
-      tcImports: TcImports,
-      tcGlobals,
-      prefixPathOpt,
-      tcSink,
-      tcState: TcState,
-      input: ParsedInput,
-      skipImplIfSigExists: bool): (unit -> bool) * TcConfig * TcImports * TcGlobals * LongIdent option * TcResultsSink * TcState * ParsedInput * bool)
-    : Cancellable<PartialResult * TcState> =
-    cancellable {
-        let! partialResult, tcState =
-            CheckOneInputAux(checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, tcSink, tcState, input, skipImplIfSigExists)
-
-        match partialResult with
-        | Choice1Of2 result -> return result, tcState
-        | Choice2Of2 (_amap, _conditionalDefines, rootSig, _priorErrors, file, tcStateForImplFile, ccuSigForFile) ->
-            let emptyImplFile = CreateEmptyDummyImplFile file.QualifiedName rootSig
-            let tcEnvAtEnd = tcStateForImplFile.TcEnvFromImpls
-            return (tcEnvAtEnd, EmptyTopAttrs, Some emptyImplFile, ccuSigForFile), tcState
+            return (tcState.TcEnvFromSignatures, EmptyTopAttrs, None, tcState.tcsCcuSig), tcState
     }
 
 // Within a file, equip loggers to locally filter w.r.t. scope pragmas in each input
@@ -1384,7 +1375,7 @@ let DiagnosticsLoggerForInput (tcConfig: TcConfig, input: ParsedInput, oldLogger
     GetDiagnosticsLoggerFilteringByScopedPragmas(false, input.ScopedPragmas, tcConfig.diagnosticsOptions, oldLogger)
 
 /// Typecheck a single file (or interactive entry into F# Interactive)
-let CheckOneInputEntry (ctok, checkForErrors, tcConfig: TcConfig, tcImports, tcGlobals, prefixPathOpt, skipImplIfSigExists) tcState input =
+let CheckOneInputEntry (ctok, checkForErrors, tcConfig: TcConfig, tcImports, tcGlobals, prefixPathOpt) tcState input =
     // Equip loggers to locally filter w.r.t. scope pragmas in each input
     use _ =
         UseTransformedDiagnosticsLogger(fun oldLogger -> DiagnosticsLoggerForInput(tcConfig, input, oldLogger))
@@ -1393,7 +1384,7 @@ let CheckOneInputEntry (ctok, checkForErrors, tcConfig: TcConfig, tcImports, tcG
 
     RequireCompilationThread ctok
 
-    CheckOneInput(checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, TcResultsSink.NoSink, tcState, input, skipImplIfSigExists)
+    CheckOneInput(checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, TcResultsSink.NoSink, tcState, input)
     |> Cancellable.runWithoutCancellation
 
 /// Finish checking multiple files (or one interactive entry into F# Interactive)
@@ -1411,7 +1402,7 @@ let CheckMultipleInputsFinish (results, tcState: TcState) =
 
 let CheckOneInputAndFinish (checkForErrors, tcConfig: TcConfig, tcImports, tcGlobals, prefixPathOpt, tcSink, tcState, input) =
     cancellable {
-        let! result, tcState = CheckOneInput(checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, tcSink, tcState, input, false)
+        let! result, tcState = CheckOneInput(checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, tcSink, tcState, input)
         let finishedResult = CheckMultipleInputsFinish([ result ], tcState)
         return finishedResult
     }
@@ -1431,7 +1422,7 @@ let CheckClosedInputSetFinish (declaredImpls: CheckedImplFile list, tcState) =
 
 let CheckMultipleInputsSequential (ctok, checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, tcState, inputs) =
     (tcState, inputs)
-    ||> List.mapFold (CheckOneInputEntry(ctok, checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt, false))
+    ||> List.mapFold (CheckOneInputEntry(ctok, checkForErrors, tcConfig, tcImports, tcGlobals, prefixPathOpt))
 
 open FSharp.Compiler.GraphChecking
 

src/Compiler/Driver/ParseAndCheckInputs.fsi

@@ -134,6 +134,11 @@ type TcState =
 /// Get the initial type checking state for a set of inputs
 val GetInitialTcState: range * string * TcConfig * TcGlobals * TcImports * TcEnv * OpenDeclaration list -> TcState
 
+/// Returns partial type check result for skipped implementation files.
+val SkippedImplFilePlaceholder:
+    tcConfig: TcConfig * tcImports: TcImports * tcGlobals: TcGlobals * tcState: TcState * input: ParsedInput ->
+        ((TcEnv * TopAttribs * CheckedImplFile option * ModuleOrNamespaceType) * TcState) option
+
 /// Check one input, returned as an Eventually computation
 val CheckOneInput:
     checkForErrors: (unit -> bool) *
@@ -143,8 +148,7 @@ val CheckOneInput:
     prefixPathOpt: LongIdent option *
     tcSink: NameResolution.TcResultsSink *
     tcState: TcState *
-    input: ParsedInput *
-    skipImplIfSigExists: bool ->
+    input: ParsedInput ->
         Cancellable<(TcEnv * TopAttribs * CheckedImplFile option * ModuleOrNamespaceType) * TcState>
 
 /// Finish the checking of multiple inputs

src/Compiler/Service/IncrementalBuild.fsi

@@ -233,7 +233,7 @@ type internal IncrementalBuilder =
         unit ->
             NodeCode<PartialCheckResults * IL.ILAssemblyRef * ProjectAssemblyDataResult * CheckedImplFile list option>
 
-    /// Get the logical time stamp that is associated with the output of the project if it were gully built immediately
+    /// Get the logical time stamp that is associated with the output of the project if it were fully built immediately
     member GetLogicalTimeStampForProject: TimeStampCache -> DateTime
 
     /// Does the given file exist in the builder's pipeline?

tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs

@@ -10,23 +10,6 @@ open FSharp.Test.ProjectGeneration
 open FSharp.Compiler.Text
 open FSharp.Compiler.CodeAnalysis
 
-module FcsDiagnostics = FSharp.Compiler.Diagnostics.Activity
-module FscActivityNames = FSharp.Compiler.Diagnostics.ActivityNames
-
-let expectCacheHits n =
-    let events = ResizeArray()
-    let listener = 
-        new ActivityListener(
-            ShouldListenTo = (fun s -> s.Name = FscActivityNames.FscSourceName),
-            Sample = (fun _ -> ActivitySamplingResult.AllData),
-            ActivityStopped = (fun a -> events.AddRange a.Events)
-        )
-    ActivitySource.AddActivityListener listener
-    { new IDisposable with 
-        member this.Dispose() =
-            listener.Dispose()
-            Assert.Equal(n, events |> Seq.filter (fun e -> e.Name = FcsDiagnostics.Events.cacheHit) |> Seq.length) }
-
 let makeTestProject () =
     SyntheticProject.Create(
         sourceFile "First" [],
@@ -150,61 +133,3 @@ let ``Using getSource and notifications instead of filesystem`` () =
         checkFile last expectSignatureChanged
     }
 
-[<Fact>]
-let ``Using getSource and notifications instead of filesystem with parse caching`` () =
-
-    let size = 20
-
-    let project =
-        { SyntheticProject.Create() with
-            SourceFiles = [
-                sourceFile $"File%03d{0}" []
-                for i in 1..size do
-                    sourceFile $"File%03d{i}" [$"File%03d{i-1}"]
-            ]
-        }
-
-    let first = "File001"
-    let middle = $"File%03d{size / 2}"
-    let last = $"File%03d{size}"
-
-    use _ = expectCacheHits 28
-    ProjectWorkflowBuilder(project, useGetSource = true, useChangeNotifications = true, useSyntaxTreeCache = true) {
-        updateFile first updatePublicSurface
-        checkFile first expectSignatureChanged
-        checkFile last expectSignatureChanged
-        updateFile middle updatePublicSurface
-        checkFile last expectSignatureChanged
-        addFileAbove middle (sourceFile "addedFile" [first])
-        updateFile middle (addDependency "addedFile")
-        checkFile middle expectSignatureChanged
-        checkFile last expectSignatureChanged
-    }
-
-[<Fact>]
-let ``Edit file, check it, then check dependent file with parse caching`` () =
-    use _ = expectCacheHits 1
-    ProjectWorkflowBuilder(makeTestProject(), useSyntaxTreeCache = true) {
-        updateFile "First" breakDependentFiles
-        checkFile "First" expectSignatureChanged
-        saveFile "First"
-        checkFile "Second" expectErrors
-    }
-
-[<Fact>]
-let ``Edit file, don't check it, check dependent file with parse caching `` () =
-    use _ = expectCacheHits 1
-    ProjectWorkflowBuilder(makeTestProject(), useSyntaxTreeCache = true) {
-        updateFile "First" breakDependentFiles
-        saveFile "First"
-        checkFile "Second" expectErrors
-    }
-
-[<Fact>]
-let ``Parse cache not used when not enabled`` () =
-    use _ = expectCacheHits 0
-    ProjectWorkflowBuilder(makeTestProject(), useSyntaxTreeCache = false) {
-        updateFile "First" breakDependentFiles
-        saveFile "First"
-        checkFile "Second" expectErrors
-    }
\ No newline at end of file

vsintegration/tests/UnitTests/LegacyLanguageService/Tests.LanguageService.Completion.fs

@@ -4444,7 +4444,7 @@ let x = query { for bbbb in abbbbc(*D0*) do
         let file3 = OpenFile(project,"File3.fs")
         TakeCoffeeBreak(this.VS)
 
-        gpatcc.AssertExactly(notAA[file2; file3], notAA[file2;file3])
+        gpatcc.AssertExactly(notAA[file2], notAA[file2;file3])
 
     /// FEATURE: References added to the project bring corresponding new .NET and F# items into scope.
     [<Test;Category("ReproX")>]