Merge pull request #445 from dsyme/fix-mem3

Fix caching, rewrite FCS reactor queue

Author: dsyme

RELEASE_NOTES.md

@@ -1,3 +1,10 @@
+#### 1.4.1 - 
+* Add pause before backgrounnd work starts. The FCS request queue must be empty for 1 second before work will start
+* Write trace information about the reactor queue to the event log
+* Rewrite reactor to consistently prioritize queued work
+* Implement cancellation for queued work if it is cancelled prior to being executed
+* Adjust caching to check cache correctly if there is a gap before the request is executed
+
 #### 1.4.0.9 - 
 * FSharpType.Format fix
 * Disable maximum-memory trigger by default until use case ironed out

docs/content/queue.fsx

@@ -6,21 +6,22 @@ Compiler Services: Notes on the FSharpChecker operations queue
 
 This is a design note on the FSharpChecker component and its operations queue.  See also the notes on the [FSharpChecker caches](caches.html)
 
-FSharpChecker maintains an operations queue. Items from the FSharpChecker operations queue are currently processed 
+FSharpChecker maintains an operations queue. Items from the FSharpChecker operations queue are processed 
 sequentially and in order. 
 
-In addition to operations in the queue, there is also a low-priority, interleaved background operation
-to bring the checking of the checking of "the current background project" up-to-date.  When the queue is empty
-this operation is conducted in small incremental fragments of parse/check work (cooperatively time-sliced to be approximately <50ms, 
-see `maxTimeShareMilliseconds` in IncrementalBuild.fs). For a working definition of "current background project" 
-you currently  have to see the calls to  `StartBackgroundCompile` 
-in [service.fs](https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/vs/service.fs),
-where an API call which in turn calls StartBackgroundCompile indicates that the current background project has been specified.
+The thread processing these requests can also run a low-priority, interleaved background operation when the
+queue is empty.  This can be used to implicitly bring the background check of a project "up-to-date".  
+When the operations queue has been empty for 1 second,
+this background work is run in small incremental fragments. This work is cooperatively time-sliced to be approximately <50ms, (see `maxTimeShareMilliseconds` in 
+IncrementalBuild.fs). The project to be checked in the background is set implicitly 
+by calls to ``CheckFileInProject`` and ``ParseAndCheckFileInProject``.
+To disable implicit background checking completely, set ``checker.ImplicitlyStartBackgroundWork`` to false.
+To change the time before background work starts, set ``checker.PauseBeforeBackgroundWork`` to the required number of milliseconds.
 
-Many calls to the FSharpChecker API enqueue an operation in the FSharpChecker compiler queue. These correspond to the 
+Most calls to the FSharpChecker API enqueue an operation in the FSharpChecker compiler queue. These correspond to the 
 calls to EnqueueAndAwaitOpAsync in [service.fs](https://github.com/fsharp/FSharp.Compiler.Service/blob/master/src/fsharp/vs/service.fs).
 
-* For example, calling `ParseAndCheckProject` enqueues a `ParseAndCheckProjectImpl` operation. The length of the 
+* For example, calling `ParseAndCheckProject` enqueues a `ParseAndCheckProjectImpl` operation. The time taken for the 
   operation will depend on how much work is required to bring the project analysis up-to-date.
 
 * Likewise, calling any of `GetUsesOfSymbol`, `GetAllUsesOfAllSymbols`, `ParseFileInProject`, 
@@ -30,30 +31,25 @@ calls to EnqueueAndAwaitOpAsync in [service.fs](https://github.com/fsharp/FSharp
   vary - many will be very fast - but they won't be processed until other operations already in the queue are complete.
 
 Some operations do not enqueue anything on the FSharpChecker operations queue - notably any accesses to the Symbol APIs.
-These are multi-threaded access to the TAST data produced by other FSharpChecker operations.
-
-In advanced interactive scenarios you may need to consider contributing design changes and extensions to how the FSharpChecker queue works, 
-in order to better serve your needs. Some tools throw a lot of interactive work at the FSharpChecker operations queue. 
-If you are writing such a component, we encourage you to get more involved in making carefully considered 
-changes, cleanup and documentation to how the queue works, to ensure it is suitable for your needs. 
-
-For example, from the FCS point of view, it is plausible to consider changing of extending how the 
-FSharpChecker operations queue works - the design of the queue and its processing is not sacred. 
-In theory you could submit extensions and documentation that allow prioritization of operations, or interleaving of 
-operations, or cancellation of operations, or more explicitness about the "current background project", and so on. 
-
-Finally, for those writing interactive editors which use FCS, in the current state of affairs you 
-should be cautious about having any auto-operation  (e.g. operations like "Find Unused Declarations" 
-which run automatically and are unassociated with a user action, but not operations like "Find All References" 
-which a user explicitly triggers) request a check of  the entire project. That will cause very long operations 
-and contention on the FSharpChecker operations queue. Any such very-long-running request should normally be associated 
-with a user action, and preferably it should be cancellable. 
-
-Alternatively, it is reasonable to trigger these 
-kinds of auto-operations when the ProjectChecked event is triggered on FSharpChecker. This event indicates the 
-completion of interleaved checking of the "current background project" (specified above). Again, if 
-the spec of "current background project" is not suitable or sufficiently controlled for your needs, please 
-help to document it and adjust it.
+These use cross-threaded access to the TAST data produced by other FSharpChecker operations.
+
+Some tools throw a lot of interactive work at the FSharpChecker operations queue. 
+If you are writing such a component, consider running your project against a debug build
+of FSharp.Compiler.Service.dll to see the Trace.WriteInformation messages indicating the length of the
+operations queuea and the time to process requests.
+
+For those writing interactive editors which use FCS, you 
+should be cautious about operations that request a check of  the entire project.
+For example, be careful about requesting the check of an entire project
+on operations like "Highlight Symbol" or "Find Unused Declarations" 
+(which run automatically when the user opens a file or moves the cursor).
+as opposed to operations like "Find All References" (which a user explicitly triggers). 
+Project checking can cause long and contention on the FSharpChecker operations queue. 
+
+Requests to FCS can be cancelled by cancelling the async operation. (Some requests also 
+include additional callbacks which can be used to indicate a cancellation condition).  
+This cancellation will be effective if the cancellation is performed before the operation 
+is executed in the operations queue.
 
 Summary
 -------

src/fsharp/FSharp.Compiler.Service/FSharp.Compiler.Service.fsproj

@@ -39,6 +39,7 @@
     <DefineConstants>$(DefineConstants);COMPILER</DefineConstants>
     <DefineConstants>$(DefineConstants);EXTENSIONTYPING</DefineConstants>
     <DefineConstants>$(DefineConstants);NO_STRONG_NAMES</DefineConstants>
+    <DefineConstants>$(DefineConstants);TRACE</DefineConstants>
     <TargetFSharpCoreVersion>4.3.0.0</TargetFSharpCoreVersion>
     <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\..\</SolutionDir>
     <TargetFrameworkProfile />

src/fsharp/fsc.fs

@@ -615,15 +615,15 @@ type ILResource with
         | ILResourceLocation.Local b -> b()
         | _-> error(InternalError("Bytes",rangeStartup))
 
-let EncodeInterfaceData(tcConfig:TcConfig,tcGlobals,exportRemapping,generatedCcu,outfile) = 
+let EncodeInterfaceData(tcConfig:TcConfig,tcGlobals,exportRemapping,generatedCcu,outfile,isIncrementalBuild) = 
       if GenerateInterfaceData(tcConfig) then 
         if verbose then dprintfn "Generating interface data attribute...";
         let resource = WriteSignatureData (tcConfig,tcGlobals,exportRemapping,generatedCcu,outfile)
         if verbose then dprintf "Generated interface data attribute!\n";
         // REVIEW: need a better test for this
         let outFileNoExtension = Filename.chopExtension outfile
         let isCompilerServiceDll = outFileNoExtension.Contains("FSharp.LanguageService.Compiler")
-        if tcConfig.useOptimizationDataFile || tcGlobals.compilingFslib || isCompilerServiceDll then 
+        if (tcConfig.useOptimizationDataFile || tcGlobals.compilingFslib || isCompilerServiceDll) && not isIncrementalBuild then 
             let sigDataFileName = (Filename.chopExtension outfile)+".sigdata"
             File.WriteAllBytes(sigDataFileName,resource.Bytes);
         let sigAttr = mkSignatureDataVersionAttr tcGlobals (IL.parseILVersion Internal.Utilities.FSharpEnvironment.FSharpBinaryMetadataFormatRevision) 
@@ -1917,7 +1917,7 @@ let main2(Args(tcConfig, tcImports, frameworkTcImports: TcImports, tcGlobals, er
 
     let sigDataAttributes,sigDataResources = 
       try
-        EncodeInterfaceData(tcConfig, tcGlobals, exportRemapping, generatedCcu, outfile)
+        EncodeInterfaceData(tcConfig, tcGlobals, exportRemapping, generatedCcu, outfile, false)
       with e -> 
         errorRecoveryNoRange e
         SqmLoggerWithConfig tcConfig errorLogger.ErrorNumbers errorLogger.WarningNumbers

src/fsharp/fsc.fsi

@@ -21,7 +21,7 @@ type ErrorLoggerProvider =
 
 type SigningInfo = SigningInfo of (* delaysign:*) bool * (*signer:*)  string option * (*container:*) string option
 
-val EncodeInterfaceData: tcConfig:TcConfig * tcGlobals:TcGlobals * exportRemapping:Tastops.Remap * generatedCcu: Tast.CcuThunk * outfile: string -> ILAttribute list * ILResource list
+val EncodeInterfaceData: tcConfig:TcConfig * tcGlobals:TcGlobals * exportRemapping:Tastops.Remap * generatedCcu: Tast.CcuThunk * outfile: string * isIncrementalBuild: bool -> ILAttribute list * ILResource list
 val ValidateKeySigningAttributes : tcConfig:TcConfig * tcGlobals:TcGlobals * TypeChecker.TopAttribs -> SigningInfo
 val GetSigner : SigningInfo -> ILBinaryWriter.ILStrongNameSigner option
 

src/fsharp/vs/IncrementalBuild.fs

@@ -1080,7 +1080,7 @@ module internal IncrementalFSharpBuild =
 #if SILVERLIGHT
         50L
 #else
-        match System.Environment.GetEnvironmentVariable("mFSharp_MaxTimeShare") with 
+        match System.Environment.GetEnvironmentVariable("FCS_MaxTimeShare") with 
         | null | "" -> 50L
         | s -> int64 s
 #endif
@@ -1500,7 +1500,7 @@ module internal IncrementalFSharpBuild =
                         let exportRemapping = MakeExportRemapping generatedCcu generatedCcu.Contents
 
                         let sigData = 
-                            let _sigDataAttributes,sigDataResources = Driver.EncodeInterfaceData(tcConfig,tcGlobals,exportRemapping,generatedCcu,outfile)
+                            let _sigDataAttributes,sigDataResources = Driver.EncodeInterfaceData(tcConfig,tcGlobals,exportRemapping,generatedCcu,outfile,true)
                             [ for r in sigDataResources  do
                                 let ccuName = GetSignatureDataResourceName r
                                 let bytes = 

src/fsharp/vs/IncrementalBuild.fsi

@@ -107,7 +107,7 @@ module internal IncrementalFSharpBuild =
       /// Whether there are any 'live' type providers that may need a refresh when a project is Cleaned
       member ThereAreLiveTypeProviders : bool
 #endif
-      /// Perform one step in the F# build (type-checking only, the type check is not finalized)
+      /// Perform one step in the F# build. Return true if the background work is finished.
       member Step : unit -> bool
 
       /// Get the preceding typecheck state of a slot, without checking if it is up-to-date w.r.t.