Skip to content

Commit

Permalink
Add a version of TryGetRecentCheckResultsForFile with project snapshot (
Browse files Browse the repository at this point in the history
dotnet#16720)

* First stab at adding a version of TryGetRecentCheckResultsForFile that takes a snapshot instead of project options

* use version type without defaultof<_> = null

* take sourcetext out of project snapshot, kind of forces us to an async return but let's see if this is acceptable

* format

* - reuse ParseAndCheckFileInProject cache for TryGetRecentCheckResultsForFile
- extend version of ParseAndCheckFileInProject cache with the check sum of the source code
- add test

* format

* cleanup

* use a new LruCache member GetAll to get rid of the dummy version

* cleanup

* Update src/Compiler/Service/TransparentCompiler.fs

Co-authored-by: Petr Pokorny <petr@innit.cz>

* unify key creation

* just use ProjectSnapShot.FileKey

* to have it on record, push a version with "f.Version |> Md5Hasher.toString" as the second part of lastFileKey.Version

* use FileKeyWithExtraFileSnapshotVersion for the ParseAndCheckFileInProject cache

* replace FileSnapShot after edit in Test

* add CustomOperation tryGetRecentCheckResults for tests

* - Make API non-async and don't return hash
- let tests run for background compiler, too

* better fix for commandLineOptions order

* Update src/Compiler/Service/FSharpProjectSnapshot.fs

Co-authored-by: Petr Pokorny <petr@innit.cz>

* Update tests/FSharp.Compiler.ComponentTests/FSharpChecker/TransparentCompiler.fs

Co-authored-by: Petr Pokorny <petr@innit.cz>

* Update tests/FSharp.Compiler.ComponentTests/FSharpChecker/TransparentCompiler.fs

Co-authored-by: Petr Pokorny <petr@innit.cz>

* fix version predicate

* let LruCache.GetAll(key: 'TKey) return a seq instead of a list

* compare signatures in CustomOperation tryGetRecentCheckResults to tighten the tests

---------

Co-authored-by: Petr Pokorny <petr@innit.cz>
  • Loading branch information
2 people authored and nojaf committed Feb 26, 2024
1 parent 32e3b0a commit 39e8490
Show file tree
Hide file tree
Showing 16 changed files with 274 additions and 32 deletions.
9 changes: 9 additions & 0 deletions src/Compiler/Facilities/AsyncMemoize.fs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,15 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T

}

member _.TryGet(key: 'TKey, predicate: 'TVersion -> bool) : 'TValue option =
let versionsAndJobs = cache.GetAll(key)

versionsAndJobs
|> Seq.tryPick (fun (version, job) ->
match predicate version, job with
| true, Completed(completed, _) -> Some completed
| _ -> None)

member _.Clear() = cache.Clear()

member _.Clear predicate = cache.Clear predicate
Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/Facilities/AsyncMemoize.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ type internal AsyncMemoize<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'T

member Get': key: 'TKey * computation: NodeCode<'TValue> -> NodeCode<'TValue>

member TryGet: key: 'TKey * predicate: ('TVersion -> bool) -> 'TValue option

member Event: IEvent<JobEvent * (string * 'TKey * 'TVersion)>

member OnEvent: ((JobEvent * (string * 'TKey * 'TVersion) -> unit) -> unit)
Expand Down
24 changes: 23 additions & 1 deletion src/Compiler/Service/BackgroundCompiler.fs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ type internal IBackgroundCompiler =

abstract GetCachedScriptSnapshot: path: string -> FSharpProjectSnapshot option

abstract member TryGetRecentCheckResultsForFile:
fileName: string * projectSnapshot: FSharpProjectSnapshot * userOpName: string ->
(FSharpParseFileResults * FSharpCheckFileResults) option

abstract member BeforeBackgroundFileCheck: IEvent<string * FSharpProjectOptions>

abstract member FileChecked: IEvent<string * FSharpProjectOptions>
Expand Down Expand Up @@ -1176,6 +1180,16 @@ type internal BackgroundCompiler
| None -> None
| None -> None

member _.TryGetRecentCheckResultsForFile(fileName: string, projectSnapshot: FSharpProjectSnapshot, userOpName: string) =
projectSnapshot.ProjectSnapshot.SourceFiles
|> List.tryFind (fun f -> f.FileName = fileName)
|> Option.bind (fun (f: FSharpFileSnapshot) ->
let options = projectSnapshot.ToOptions()
let sourceText = f.GetSource() |> Async.AwaitTask |> Async.RunSynchronously

self.TryGetRecentCheckResultsForFile(fileName, options, Some sourceText, userOpName)
|> Option.map (fun (parseFileResults, checkFileResults, _hash) -> (parseFileResults, checkFileResults)))

/// Parse and typecheck the whole project (the implementation, called recursively as project graph is evaluated)
member private _.ParseAndCheckProjectImpl(options, userOpName) =
node {
Expand Down Expand Up @@ -1641,7 +1655,7 @@ type internal BackgroundCompiler
userOpName
)

let! snapshot = FSharpProjectSnapshot.FromOptions(options, DocumentSource.FileSystem)
let! snapshot = FSharpProjectSnapshot.FromOptions(options)
return snapshot, diagnostics
}

Expand Down Expand Up @@ -1742,3 +1756,11 @@ type internal BackgroundCompiler
// I'm not expecting anything actually async to happen here.
// As the snapshot will most likely not have any referenced projects
FSharpProjectSnapshot.FromOptions options |> Async.RunSynchronously)

member _.TryGetRecentCheckResultsForFile
(
fileName: string,
projectSnapshot: FSharpProjectSnapshot,
userOpName: string
) : (FSharpParseFileResults * FSharpCheckFileResults) option =
self.TryGetRecentCheckResultsForFile(fileName, projectSnapshot, userOpName)
4 changes: 4 additions & 0 deletions src/Compiler/Service/BackgroundCompiler.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,10 @@ type internal IBackgroundCompiler =

abstract GetCachedScriptSnapshot: path: string -> FSharpProjectSnapshot option

abstract TryGetRecentCheckResultsForFile:
fileName: string * projectSnapshot: FSharpProjectSnapshot * userOpName: string ->
(FSharpParseFileResults * FSharpCheckFileResults) option

abstract BeforeBackgroundFileCheck: IEvent<string * FSharpProjectOptions>

abstract FileChecked: IEvent<string * FSharpProjectOptions>
Expand Down
6 changes: 6 additions & 0 deletions src/Compiler/Service/FSharpCheckerResults.fs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ open System.Threading.Tasks
open System.Runtime.CompilerServices
open Internal.Utilities.Hashing

[<Experimental "This type is experimental and likely to be removed in the future.">]
[<RequireQualifiedAccess>]
type DocumentSource =
| FileSystem
| Custom of (string -> Async<ISourceText option>)

type FSharpUnresolvedReferencesSet = FSharpUnresolvedReferencesSet of UnresolvedAssemblyReference list

[<Sealed>]
Expand Down
6 changes: 6 additions & 0 deletions src/Compiler/Service/FSharpCheckerResults.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ open FSharp.Compiler.Text

open Internal.Utilities.Collections

[<Experimental "This type is experimental and likely to be removed in the future.">]
[<RequireQualifiedAccess>]
type DocumentSource =
| FileSystem
| Custom of (string -> Async<ISourceText option>)

/// Delays the creation of an ILModuleReader
[<Sealed>]
type DelayedILModuleReader =
Expand Down
10 changes: 8 additions & 2 deletions src/Compiler/Service/FSharpProjectSnapshot.fs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ type internal ProjectSnapshotBase<'T when 'T :> IFileSnapshot>(projectCore: Proj
member this.FileKey(fileName: string) = this.UpTo(fileName).LastFileKey
member this.FileKey(index: FileIndex) = this.UpTo(index).LastFileKey

member this.FileKeyWithExtraFileSnapshotVersion(fileName: string) =
let fileKey = this.FileKey fileName
let fileSnapshot = this.SourceFiles |> Seq.find (fun f -> f.FileName = fileName)

fileKey.WithExtraVersion(fileSnapshot.Version |> Md5Hasher.toString)

/// Project snapshot with filenames and versions given as initial input
and internal ProjectSnapshot = ProjectSnapshotBase<FSharpFileSnapshot>

Expand Down Expand Up @@ -375,10 +381,10 @@ and internal ProjectCore
let commandLineOptions =
lazy
(seq {
yield! OtherOptions

for r in ReferencesOnDisk do
$"-r:{r.Path}"

yield! OtherOptions
}
|> Seq.toList)

Expand Down
71 changes: 59 additions & 12 deletions src/Compiler/Service/TransparentCompiler.fs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ type internal CompilerCaches(sizeFactor: int) =
this.AssemblyData.Clear(shouldClear)
this.SemanticClassification.Clear(snd >> shouldClear)
this.ItemKeyStore.Clear(snd >> shouldClear)
this.ScriptClosure.Clear(snd >> shouldClear) // Todo check if correct predicate
this.ScriptClosure.Clear(snd >> shouldClear)

type internal TransparentCompiler
(
Expand Down Expand Up @@ -1467,9 +1467,8 @@ type internal TransparentCompiler

let ComputeParseAndCheckFileInProject (fileName: string) (projectSnapshot: ProjectSnapshot) =
caches.ParseAndCheckFileInProject.Get(
projectSnapshot.FileKey fileName,
projectSnapshot.FileKeyWithExtraFileSnapshotVersion fileName,
node {

use _ =
Activity.start "ComputeParseAndCheckFileInProject" [| Activity.Tags.fileName, fileName |> Path.GetFileName |]

Expand All @@ -1496,8 +1495,6 @@ type internal TransparentCompiler
let tcSymbolUses = sink.GetSymbolUses()
let tcOpenDeclarations = sink.GetOpenDeclarations()

let tcDependencyFiles = [] // TODO add as a set to TcIntermediate

// TODO: Apparently creating diagnostics can produce further diagnostics. So let's capture those too. Hopefully there is a more elegant solution...
// Probably diagnostics need to be evaluated during typecheck anyway for proper formatting, which might take care of this too.
let extraLogger = CapturingDiagnosticsLogger("DiagnosticsWhileCreatingDiagnostics")
Expand Down Expand Up @@ -1559,7 +1556,7 @@ type internal TransparentCompiler
projectSnapshot.IsIncompleteTypeCheckEnvironment,
None,
projectSnapshot.ToOptions(),
Array.ofList tcDependencyFiles,
Array.ofList tcInfo.tcDependencyFiles,
creationDiags,
parseResults.Diagnostics,
tcDiagnostics,
Expand Down Expand Up @@ -1602,6 +1599,29 @@ type internal TransparentCompiler
}
)

let TryGetRecentCheckResultsForFile
(
fileName: string,
projectSnapshot: FSharpProjectSnapshot,
userOpName: string
) : (FSharpParseFileResults * FSharpCheckFileResults) option =
ignore userOpName

let cacheKey =
projectSnapshot.ProjectSnapshot.FileKeyWithExtraFileSnapshotVersion fileName

let version = cacheKey.GetVersion()

let parseFileResultsAndcheckFileAnswer =
caches.ParseAndCheckFileInProject.TryGet(
cacheKey.GetKey(),
(fun (_fullVersion, fileContentVersion) -> fileContentVersion = (snd version))
)

match parseFileResultsAndcheckFileAnswer with
| Some(parseFileResults, FSharpCheckFileAnswer.Succeeded checkFileResults) -> Some(parseFileResults, checkFileResults)
| _ -> None

let ComputeProjectExtras (bootstrapInfo: BootstrapInfo) (projectSnapshot: ProjectSnapshotWithSources) =
caches.ProjectExtras.Get(
projectSnapshot.SignatureKey,
Expand Down Expand Up @@ -2030,7 +2050,10 @@ type internal TransparentCompiler
) : NodeCode<seq<range>> =
node {
ignore canInvalidateProject
let! snapshot = FSharpProjectSnapshot.FromOptions options |> NodeCode.AwaitAsync

let! snapshot =
FSharpProjectSnapshot.FromOptions(options)
|> NodeCode.AwaitAsync

return! this.FindReferencesInFile(fileName, snapshot.ProjectSnapshot, symbol, userOpName)
}
Expand All @@ -2043,7 +2066,10 @@ type internal TransparentCompiler

member this.GetAssemblyData(options: FSharpProjectOptions, fileName, userOpName: string) : NodeCode<ProjectAssemblyDataResult> =
node {
let! snapshot = FSharpProjectSnapshot.FromOptions options |> NodeCode.AwaitAsync
let! snapshot =
FSharpProjectSnapshot.FromOptions(options)
|> NodeCode.AwaitAsync

return! this.GetAssemblyData(snapshot.ProjectSnapshot, fileName, userOpName)
}

Expand All @@ -2062,7 +2088,9 @@ type internal TransparentCompiler
userOpName: string
) : NodeCode<FSharpParseFileResults * FSharpCheckFileResults> =
node {
let! snapshot = FSharpProjectSnapshot.FromOptions options |> NodeCode.AwaitAsync
let! snapshot =
FSharpProjectSnapshot.FromOptions(options)
|> NodeCode.AwaitAsync

match! this.ParseAndCheckFileInProject(fileName, snapshot.ProjectSnapshot, userOpName) with
| parseResult, FSharpCheckFileAnswer.Succeeded checkResult -> return parseResult, checkResult
Expand All @@ -2076,7 +2104,10 @@ type internal TransparentCompiler
userOpName: string
) : NodeCode<FSharpParseFileResults> =
node {
let! snapshot = FSharpProjectSnapshot.FromOptions options |> NodeCode.AwaitAsync
let! snapshot =
FSharpProjectSnapshot.FromOptions(options)
|> NodeCode.AwaitAsync

return! this.ParseFile(fileName, snapshot.ProjectSnapshot, userOpName)
}

Expand Down Expand Up @@ -2260,7 +2291,11 @@ type internal TransparentCompiler
) : NodeCode<EditorServices.SemanticClassificationView option> =
node {
ignore userOpName
let! snapshot = FSharpProjectSnapshot.FromOptions options |> NodeCode.AwaitAsync

let! snapshot =
FSharpProjectSnapshot.FromOptions(options)
|> NodeCode.AwaitAsync

return! ComputeSemanticClassification(fileName, snapshot.ProjectSnapshot)
}

Expand Down Expand Up @@ -2301,7 +2336,11 @@ type internal TransparentCompiler
member this.ParseAndCheckProject(options: FSharpProjectOptions, userOpName: string) : NodeCode<FSharpCheckProjectResults> =
node {
ignore userOpName
let! snapshot = FSharpProjectSnapshot.FromOptions options |> NodeCode.AwaitAsync

let! snapshot =
FSharpProjectSnapshot.FromOptions(options)
|> NodeCode.AwaitAsync

return! ComputeParseAndCheckProject snapshot.ProjectSnapshot
}

Expand Down Expand Up @@ -2336,3 +2375,11 @@ type internal TransparentCompiler
backgroundCompiler.TryGetRecentCheckResultsForFile(fileName, options, sourceText, userOpName)

member this.GetCachedScriptSnapshot _ = None

member this.TryGetRecentCheckResultsForFile
(
fileName: string,
projectSnapshot: FSharpProjectSnapshot,
userOpName: string
) : (FSharpParseFileResults * FSharpCheckFileResults) option =
TryGetRecentCheckResultsForFile(fileName, projectSnapshot, userOpName)
2 changes: 1 addition & 1 deletion src/Compiler/Service/TransparentCompiler.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ type internal CompilerCaches =
member ParseAndCheckAllFilesInProject: AsyncMemoizeDisabled<obj, obj, obj>

member ParseAndCheckFileInProject:
AsyncMemoize<(string * (string * string)), string, (FSharpParseFileResults * FSharpCheckFileAnswer)>
AsyncMemoize<(string * (string * string)), string * string, (FSharpParseFileResults * FSharpCheckFileAnswer)>

member ParseAndCheckProject: AsyncMemoize<(string * string), string, FSharpCheckProjectResults>

Expand Down
9 changes: 4 additions & 5 deletions src/Compiler/Service/service.fs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ open FSharp.Compiler.Text.Range
open FSharp.Compiler.TcGlobals
open FSharp.Compiler.BuildGraph

[<RequireQualifiedAccess>]
type DocumentSource =
| FileSystem
| Custom of (string -> Async<ISourceText option>)

/// Callback that indicates whether a requested result has become obsolete.
[<NoComparison; NoEquality>]
type IsResultObsolete = IsResultObsolete of (unit -> bool)
Expand Down Expand Up @@ -346,6 +341,10 @@ type FSharpChecker
let userOpName = defaultArg userOpName "Unknown"
backgroundCompiler.TryGetRecentCheckResultsForFile(fileName, options, sourceText, userOpName)

member _.TryGetRecentCheckResultsForFile(fileName: string, projectSnapshot: FSharpProjectSnapshot, ?userOpName: string) =
let userOpName = defaultArg userOpName "Unknown"
backgroundCompiler.TryGetRecentCheckResultsForFile(fileName, projectSnapshot, userOpName)

member _.Compile(argv: string[], ?userOpName: string) =
let _userOpName = defaultArg userOpName "Unknown"
use _ = Activity.start "FSharpChecker.Compile" [| Activity.Tags.userOpName, _userOpName |]
Expand Down
11 changes: 5 additions & 6 deletions src/Compiler/Service/service.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ open FSharp.Compiler.Syntax
open FSharp.Compiler.Text
open FSharp.Compiler.Tokenization

[<Experimental "This type is experimental and likely to be removed in the future.">]
[<RequireQualifiedAccess>]
type DocumentSource =
| FileSystem
| Custom of (string -> Async<ISourceText option>)

/// Used to parse and check F# source code.
[<Sealed; AutoSerializable(false)>]
type public FSharpChecker =
Expand Down Expand Up @@ -429,6 +423,11 @@ type public FSharpChecker =
fileName: string * options: FSharpProjectOptions * ?sourceText: ISourceText * ?userOpName: string ->
(FSharpParseFileResults * FSharpCheckFileResults (* hash *) * int64) option

[<Experimental("This FCS API is experimental and subject to change.")>]
member TryGetRecentCheckResultsForFile:
fileName: string * projectSnapshot: FSharpProjectSnapshot * ?userOpName: string ->
(FSharpParseFileResults * FSharpCheckFileResults) option

/// This function is called when the entire environment is known to have changed for reasons not encoded in the ProjectOptions of any project/compilation.
member InvalidateAll: unit -> unit

Expand Down
14 changes: 10 additions & 4 deletions src/Compiler/Utilities/LruCache.fs
Original file line number Diff line number Diff line change
Expand Up @@ -198,21 +198,27 @@ type internal LruCache<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'TVers

/// Returns an option of a value for given key and version, and also a list of all other versions for given key
member this.GetAll(key, version) =
this.TryGet(key, version),
let others =
this.GetAll(key) |> Seq.filter (fun (ver, _val) -> ver <> version) |> Seq.toList

this.TryGet(key, version), others

/// Returns a list of version * value pairs for a given key. The strongly held value is first in the list.
member _.GetAll(key: 'TKey) : ('TVersion * 'TValue) seq =
match dictionary.TryGetValue key with
| false, _ -> []
| true, versionDict ->
versionDict.Values
|> Seq.map (fun node -> node.Value)
|> Seq.filter (p24 >> ((<>) version))
|> Seq.map (_.Value)
|> Seq.sortBy (function
| _, _, _, Strong _ -> 0
| _ -> 1)
|> Seq.choose (function
| _, ver, _, Strong v -> Some(ver, v)
| _, ver, _, Weak r ->
match r.TryGetTarget() with
| true, x -> Some(ver, x)
| _ -> None)
|> Seq.toList

member _.Remove(key, version) =
match dictionary.TryGetValue key with
Expand Down
3 changes: 3 additions & 0 deletions src/Compiler/Utilities/LruCache.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ type internal LruCache<'TKey, 'TVersion, 'TValue when 'TKey: equality and 'TVers
/// Returns an option of a value for given key and version, and also a list of all other versions for given key
member GetAll: key: 'TKey * version: 'TVersion -> 'TValue option * ('TVersion * 'TValue) list

/// Returns a list of version * value pairs for a given key. The strongly held value is first in the list.
member GetAll: key: 'TKey -> ('TVersion * 'TValue) seq

member GetValues: unit -> (string * 'TVersion * 'TValue) seq

member Remove: key: 'TKey -> unit
Expand Down
Loading

0 comments on commit 39e8490

Please sign in to comment.