Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix solution reload and config change issues #3025

Merged
merged 20 commits into from
May 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type ProjectCracker =

logMap := Map.add opts.ProjectFile opts.LogOutput !logMap
{ ProjectFileName = opts.ProjectFile
ProjectFileNames = sourceFiles
SourceFiles = sourceFiles
OtherOptions = otherOptions
ReferencedProjects = referencedProjects
IsIncompleteTypeCheckEnvironment = false
Expand Down
2 changes: 1 addition & 1 deletion src/absil/ilread.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3967,7 +3967,7 @@ let OpenILModuleReader infile opts =

// ++GLOBAL MUTABLE STATE (concurrency safe via locking)
type ILModuleReaderCacheLockToken() = interface LockToken
let ilModuleReaderCache = new AgedLookup<ILModuleReaderCacheLockToken, (string * System.DateTime * ILScopeRef * bool), ILModuleReader>(0, areSame=(fun (x,y) -> x = y))
let ilModuleReaderCache = new AgedLookup<ILModuleReaderCacheLockToken, (string * System.DateTime * ILScopeRef * bool), ILModuleReader>(0, areSimilar=(fun (x,y) -> x = y))
let ilModuleReaderCacheLock = Lock()

let OpenILModuleReaderAfterReadingAllBytes infile opts =
Expand Down
16 changes: 9 additions & 7 deletions src/fsharp/CompileOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2701,13 +2701,15 @@ type TcConfig private (data : TcConfigBuilder,validate:bool) =
match fslibExplicitFilenameOpt with
| Some(fslibFilename) ->
let filename = ComputeMakePathAbsolute data.implicitIncludeDir fslibFilename
try
use ilReader = OpenILBinary(filename,data.optimizeForMemory,data.openBinariesInMemory,None,None, data.shadowCopyReferences)
checkFSharpBinaryCompatWithMscorlib filename ilReader.ILAssemblyRefs ilReader.ILModuleDef.ManifestOfAssembly.Version rangeStartup;
let fslibRoot = Path.GetDirectoryName(FileSystem.GetFullPathShim(filename))
fslibRoot (* , sprintf "v%d.%d" v1 v2 *)
with e ->
error(Error(FSComp.SR.buildErrorOpeningBinaryFile(filename, e.Message), rangeStartup))
if fslibReference.ProjectReference.IsNone then
try
use ilReader = OpenILBinary(filename,data.optimizeForMemory,data.openBinariesInMemory,None,None, data.shadowCopyReferences)
checkFSharpBinaryCompatWithMscorlib filename ilReader.ILAssemblyRefs ilReader.ILModuleDef.ManifestOfAssembly.Version rangeStartup;
with e ->
error(Error(FSComp.SR.buildErrorOpeningBinaryFile(filename, e.Message), rangeStartup))

let fslibRoot = Path.GetDirectoryName(FileSystem.GetFullPathShim(filename))
fslibRoot
| _ ->
data.defaultFSharpBinariesDir
#endif
Expand Down
37 changes: 21 additions & 16 deletions src/fsharp/InternalCollections.fs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type internal ValueStrength<'T when 'T : not struct> =
| Weak of WeakReference<'T>
#endif

type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStrongly:int, areSame, ?requiredToKeep, ?onStrongDiscard, ?keepMax: int) =
type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStrongly:int, areSimilar, ?requiredToKeep, ?onStrongDiscard, ?keepMax: int) =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just renaming arguments for consistency

/// The list of items stored. Youngest is at the end of the list.
/// The choice of order is somewhat arbitrary. If the other way then adding
/// items would be O(1) and removing O(N).
Expand All @@ -39,8 +39,8 @@ type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStro
// This function returns true if two keys are the same according to the predicate
// function passed in.
| []->None
| (key',value)::t->
if areSame(key,key') then Some(key',value)
| (similarKey,value)::t->
if areSimilar(key,similarKey) then Some(similarKey,value)
else Lookup key t
Lookup key data

Expand All @@ -53,18 +53,18 @@ type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStro

/// Promote a particular key value.
let Promote (data, key, value) =
(data |> List.filter (fun (key',_)-> not (areSame(key,key')))) @ [ (key, value) ]
(data |> List.filter (fun (similarKey,_)-> not (areSimilar(key,similarKey)))) @ [ (key, value) ]

/// Remove a particular key value.
let RemoveImpl (data, key) =
let discard,keep = data |> List.partition (fun (key',_)-> areSame(key,key'))
let discard,keep = data |> List.partition (fun (similarKey,_)-> areSimilar(key,similarKey))
keep, discard

let TryGetKeyValueImpl(data,key) =
match TryPeekKeyValueImpl(data,key) with
| Some(key', value) as result ->
| Some(similarKey, value) as result ->
// If the result existed, move it to the end of the list (more likely to keep it)
result,Promote (data,key',value)
result,Promote (data,similarKey,value)
| None -> None,data

/// Remove weak entries from the list that have been collected.
Expand Down Expand Up @@ -154,37 +154,42 @@ type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct>(keepStro



type internal MruCache<'Token, 'Key,'Value when 'Value : not struct>(keepStrongly, areSame, ?isStillValid : 'Key*'Value->bool, ?areSameForSubsumption, ?requiredToKeep, ?onStrongDiscard, ?keepMax) =
type internal MruCache<'Token, 'Key,'Value when 'Value : not struct>(keepStrongly, areSame, ?isStillValid : 'Key*'Value->bool, ?areSimilar, ?requiredToKeep, ?onStrongDiscard, ?keepMax) =

/// Default behavior of <c>areSameForSubsumption</c> function is areSame.
let areSameForSubsumption = defaultArg areSameForSubsumption areSame
/// Default behavior of <c>areSimilar</c> function is areSame.
let areSimilar = defaultArg areSimilar areSame

/// The list of items in the cache. Youngest is at the end of the list.
/// The choice of order is somewhat arbitrary. If the other way then adding
/// items would be O(1) and removing O(N).
let cache = AgedLookup<'Token, 'Key,'Value>(keepStrongly=keepStrongly,areSame=areSameForSubsumption,?onStrongDiscard=onStrongDiscard,?keepMax=keepMax,?requiredToKeep=requiredToKeep)
let cache = AgedLookup<'Token, 'Key,'Value>(keepStrongly=keepStrongly,areSimilar=areSimilar,?onStrongDiscard=onStrongDiscard,?keepMax=keepMax,?requiredToKeep=requiredToKeep)

/// Whether or not this result value is still valid.
let isStillValid = defaultArg isStillValid (fun _ -> true)

member bc.ContainsSimilarKey(tok, key) =
match cache.TryPeekKeyValue(tok, key) with
| Some(_similarKey, _value)-> true
| None -> false

member bc.TryGetAny(tok, key) =
match cache.TryPeekKeyValue(tok, key) with
| Some(key', value)->
if areSame(key',key) then Some(value)
| Some(similarKey, value)->
if areSame(similarKey,key) then Some(value)
else None
| None -> None

member bc.TryGet(tok, key) =
match cache.TryGetKeyValue(tok, key) with
| Some(key', value) ->
if areSame(key', key) && isStillValid(key,value) then Some value
| Some(similarKey, value) ->
if areSame(similarKey, key) && isStillValid(key,value) then Some value
else None
| None -> None

member bc.Set(tok, key:'Key,value:'Value) =
cache.Put(tok, key,value)

member bc.Remove(tok, key) =
member bc.RemoveAnySimilar(tok, key) =
cache.Remove(tok, key)

member bc.Clear(tok) =
Expand Down
25 changes: 19 additions & 6 deletions src/fsharp/InternalCollections.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ namespace Internal.Utilities.Collections

/// Simple aging lookup table. When a member is accessed it's
/// moved to the top of the list and when there are too many elements
/// the least-recently-accessed element falls of the end.
/// the least-recently-accessed element falls of the end.
///
/// - areSimilar: Keep at most once association for two similar keys (as given by areSimilar)
type internal AgedLookup<'Token, 'Key, 'Value when 'Value : not struct> =
new : keepStrongly:int
* areSame:('Key * 'Key -> bool)
* areSimilar:('Key * 'Key -> bool)
* ?requiredToKeep:('Value -> bool)
* ?onStrongDiscard : ('Value -> unit) // this may only be set if keepTotal=keepStrongly, i.e. not weak entries
* ?keepMax: int
Expand Down Expand Up @@ -40,25 +42,36 @@ namespace Internal.Utilities.Collections
/// threads seeing different live sets of cached items, and may result in the onDiscard action
/// being called multiple times. In practice this means the collection is only safe for concurrent
/// access if there is no discard action to execute.
///
/// - areSimilar: Keep at most once association for two similar keys (as given by areSimilar)
type internal MruCache<'Token, 'Key,'Value when 'Value : not struct> =
new : keepStrongly:int
* areSame:('Key * 'Key -> bool)
* ?isStillValid:('Key * 'Value -> bool)
* ?areSameForSubsumption:('Key * 'Key -> bool)
* ?areSimilar:('Key * 'Key -> bool)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A XML Doc comment would be great here explaining what "similarity" means here and what it will be used for.

* ?requiredToKeep:('Value -> bool)
* ?onDiscard:('Value -> unit)
* ?keepMax:int
-> MruCache<'Token,'Key,'Value>

/// Clear out the cache.
member Clear : 'Token -> unit
/// Get the value for the given key or <c>None</c> if not already available.

/// Get the similar (subsumable) value for the given key or <c>None</c> if not already available.
member ContainsSimilarKey : 'Token * key:'Key -> bool

/// Get the value for the given key or <c>None</c> if not still valid.
member TryGetAny : 'Token * key:'Key -> 'Value option
/// Get the value for the given key or None if not already available

/// Get the value for the given key or None, but only if entry is still valid
member TryGet : 'Token * key:'Key -> 'Value option

/// Remove the given value from the mru cache.
member Remove : 'Token * key:'Key -> unit
member RemoveAnySimilar : 'Token * key:'Key -> unit

/// Set the given key.
member Set : 'Token * key:'Key * value:'Value -> unit

/// Resize
member Resize : 'Token * keepStrongly: int * ?keepMax : int -> unit

Expand Down
6 changes: 3 additions & 3 deletions src/fsharp/vs/IncrementalBuild.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ type FrameworkImportsCacheKey = (*resolvedpath*)string list * string * (*TargetF
type FrameworkImportsCache(keepStrongly) =

// Mutable collection protected via CompilationThreadToken
let frameworkTcImportsCache = AgedLookup<CompilationThreadToken, FrameworkImportsCacheKey,(TcGlobals * TcImports)>(keepStrongly, areSame=(fun (x,y) -> x = y))
let frameworkTcImportsCache = AgedLookup<CompilationThreadToken, FrameworkImportsCacheKey,(TcGlobals * TcImports)>(keepStrongly, areSimilar=(fun (x,y) -> x = y))

member __.Downsize(ctok) = frameworkTcImportsCache.Resize(ctok, keepStrongly=0)
member __.Clear(ctok) = frameworkTcImportsCache.Clear(ctok)
Expand Down Expand Up @@ -1562,7 +1562,7 @@ type IncrementalBuilder(tcGlobals,frameworkTcImports, nonFrameworkAssemblyInputs
return true
}

member builder.GetCheckResultsBeforeFileInProjectIfReady (filename): PartialCheckResults option =
member builder.GetCheckResultsBeforeFileInProjectEvenIfStale (filename): PartialCheckResults option =
let slotOfFile = builder.GetSlotOfFileName filename
let result =
match slotOfFile with
Expand Down Expand Up @@ -1676,7 +1676,7 @@ type IncrementalBuilder(tcGlobals,frameworkTcImports, nonFrameworkAssemblyInputs
#endif
}

member __.ProjectFileNames = sourceFiles |> List.map (fun (_,f,_) -> f)
member __.SourceFiles = sourceFiles |> List.map (fun (_,f,_) -> f)

/// CreateIncrementalBuilder (for background type checking). Note that fsc.fs also
/// creates an incremental builder used by the command line compiler.
Expand Down
12 changes: 6 additions & 6 deletions src/fsharp/vs/IncrementalBuild.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,14 @@ type internal PartialCheckResults =
[<Class>]
type internal IncrementalBuilder =

/// Increment the usage count on the IncrementalBuilder by 1. This initial usage count is 0. The returns an IDisposable which will
/// decrement the usage count on the entire build by 1 and dispose if it is no longer used by anyone.
member IncrementUsageCount : unit -> IDisposable

/// Check if the builder is not disposed
member IsAlive : bool

/// The TcConfig passed in to the builder creation.
member TcConfig : TcConfig

/// The full set of source files including those from options
member ProjectFileNames : string list
member SourceFiles : string list

/// Raised just before a file is type-checked, to invalidate the state of the file in VS and force VS to request a new direct typecheck of the file.
/// The incremental builder also typechecks the file (error and intellisense results from the background builder are not
Expand Down Expand Up @@ -110,7 +106,7 @@ type internal IncrementalBuilder =
/// This is a very quick operation.
///
/// This is safe for use from non-compiler threads but the objects returned must in many cases be accessed only from the compiler thread.
member GetCheckResultsBeforeFileInProjectIfReady: filename:string -> PartialCheckResults option
member GetCheckResultsBeforeFileInProjectEvenIfStale: filename:string -> PartialCheckResults option

/// Get the preceding typecheck state of a slot, but only if it is up-to-date w.r.t.
/// the timestamps on files and referenced DLLs prior to this one. Return None if the result is not available.
Expand Down Expand Up @@ -153,7 +149,11 @@ type internal IncrementalBuilder =

static member TryCreateBackgroundBuilderForProjectOptions : CompilationThreadToken * ReferenceResolver.Resolver * defaultFSharpBinariesDir: string * FrameworkImportsCache * scriptClosureOptions:LoadClosure option * sourceFiles:string list * commandLineArgs:string list * projectReferences: IProjectReference list * projectDirectory:string * useScriptResolutionRules:bool * keepAssemblyContents: bool * keepAllBackgroundResolutions: bool * maxTimeShareMilliseconds: int64 -> Cancellable<IncrementalBuilder option * FSharpErrorInfo list>

/// Increment the usage count on the IncrementalBuilder by 1. This initial usage count is 0 so immediately after creation
/// a call to KeepBuilderAlive should be made. The returns an IDisposable which will
/// decrement the usage count and dispose if the usage count goes to zero
static member KeepBuilderAlive : IncrementalBuilder option -> IDisposable

member IsBeingKeptAliveApartFromCacheEntry : bool

/// Generalized Incremental Builder. This is exposed only for unittesting purposes.
Expand Down
Loading