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.

Sign up for GitHub

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

Jump to bottom

Use hash code instead of full source text for language service caches #5944

Closed
wants to merge 3 commits into from
Closed

Use hash code instead of full source text for language service caches #5944

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e40fd3f
Use hash code instead of full source text for language service caches
cartermp Nov 20, 2018
67a25a9
Clean diff
cartermp Nov 24, 2018
969ed66
you gotta be kidding me
cartermp Nov 24, 2018
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
46 changes: 24 additions & 22 deletions src/fsharp/service/service.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2197,17 +2197,17 @@ module Helpers =
&& 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)) =
fileName1 = fileName2 && options1 = options2 && source1 = source2
let AreSameForParsing((fileName1: string, source1Hash: int, options1), (fileName2, source2Hash, options2)) =
fileName1 = fileName2 && options1 = options2 && source1Hash = source2Hash
Copy link
Contributor

Choose a reason for hiding this comment

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

It depends how accurate the string hash codes are and how much we care about very odd behaviour on collisions.

Checking only hash codes means that when there is collision we will re-use results from some other random parse or typecheck.

So if collisions happen very rarely this may just be ok.

If hash codes are ever based on the partial hash of the string (e.g. a prefix) then this shouldn't be done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should definitely have a think about this. At least initially, I feel that if the file names are the same and the hashes are the same, it's highly unlikely we'd get some arbitrary cached value. But let's definitely make sure we feel confident on that :)


let AreSimilarForParsing((fileName1, _, _), (fileName2, _, _)) =
let AreSimilarForParsing(fileName1, fileName2) =
fileName1 = fileName2

/// Determine whether two (fileName,sourceText,options) keys should be identical w.r.t. checking
let AreSameForChecking3((fileName1: string, source1: string, options1: FSharpProjectOptions), (fileName2, source2, options2)) =
let AreSameForChecking3((fileName1: string, source1Hash: int, options1: FSharpProjectOptions), (fileName2, source2Hash, options2)) =
(fileName1 = fileName2)
&& FSharpProjectOptions.AreSameForChecking(options1,options2)
&& (source1 = source2)
&& (source1Hash = source2Hash)

/// 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)) =
Expand Down Expand Up @@ -2325,7 +2325,7 @@ module CompileHelpers =


type FileName = string
type Source = string
type SourceFileHash = int
type FilePath = string
type ProjectPath = string
type FileVersion = int
Expand Down Expand Up @@ -2470,7 +2470,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC

// Also keyed on source. This can only be out of date if the antecedent is out of date
let checkFileInProjectCache =
MruCache<ParseCacheLockToken,FileName * Source * FSharpProjectOptions, FSharpParseFileResults * FSharpCheckFileResults * FileVersion * DateTime>
MruCache<ParseCacheLockToken,FileName * SourceFileHash * FSharpProjectOptions, FSharpParseFileResults * FSharpCheckFileResults * FileVersion * DateTime>
(keepStrongly=checkFileInProjectCacheSize,
areSame=AreSameForChecking3,
areSimilar=AreSubsumable3)
Expand Down Expand Up @@ -2504,30 +2504,32 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
| Parser.TypeCheckAborted.Yes -> FSharpCheckFileAnswer.Aborted
| Parser.TypeCheckAborted.No scope -> FSharpCheckFileAnswer.Succeeded(MakeCheckFileResults(filename, options, builder, scope, dependencyFiles, creationErrors, parseErrors, tcErrors))

member bc.RecordTypeCheckFileInProjectResults(filename,options,parsingOptions,parseResults,fileVersion,priorTimeStamp,checkAnswer,source) =
member bc.RecordTypeCheckFileInProjectResults(filename,options,parsingOptions,parseResults,fileVersion,priorTimeStamp,checkAnswer,source) =
let sourceHash = source.GetHashCode()
match checkAnswer with
| None
| Some FSharpCheckFileAnswer.Aborted -> ()
| Some (FSharpCheckFileAnswer.Succeeded typedResults) ->
foregroundTypeCheckCount <- foregroundTypeCheckCount + 1
parseCacheLock.AcquireLock (fun ltok ->
checkFileInProjectCachePossiblyStale.Set(ltok, (filename,options),(parseResults,typedResults,fileVersion))
checkFileInProjectCache.Set(ltok, (filename,source,options),(parseResults,typedResults,fileVersion,priorTimeStamp))
parseFileCache.Set(ltok, (filename, source, parsingOptions), parseResults))
checkFileInProjectCache.Set(ltok, (filename,sourceHash,options),(parseResults,typedResults,fileVersion,priorTimeStamp))
parseFileCache.Set(ltok, (filename, sourceHash, parsingOptions), parseResults))

member bc.ImplicitlyStartCheckProjectInBackground(options, userOpName) =
if implicitlyStartBackgroundWork then
bc.CheckProjectInBackground(options, userOpName + ".ImplicitlyStartCheckProjectInBackground")

member bc.ParseFile(filename: string, source: string, options: FSharpParsingOptions, userOpName: string) =
let sourceHash = source.GetHashCode()
async {
match parseCacheLock.AcquireLock(fun ltok -> parseFileCache.TryGet(ltok, (filename, source, options))) with
match parseCacheLock.AcquireLock(fun ltok -> parseFileCache.TryGet(ltok, (filename, sourceHash, options))) with
| Some res -> return res
| None ->
foregroundParseCount <- foregroundParseCount + 1
let parseErrors, parseTreeOpt, anyErrors = Parser.parseFile(source, filename, options, userOpName)
let res = FSharpParseFileResults(parseErrors, parseTreeOpt, anyErrors, options.SourceFiles)
parseCacheLock.AcquireLock(fun ltok -> parseFileCache.Set(ltok, (filename, source, options), res))
parseCacheLock.AcquireLock(fun ltok -> parseFileCache.Set(ltok, (filename, sourceHash, options), res))
return res
}

Expand All @@ -2548,7 +2550,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC

member bc.GetCachedCheckFileResult(builder: IncrementalBuilder,filename,source,options) =
// Check the cache. We can only use cached results when there is no work to do to bring the background builder up-to-date
let cachedResults = parseCacheLock.AcquireLock (fun ltok -> checkFileInProjectCache.TryGet(ltok, (filename,source,options)))
let cachedResults = parseCacheLock.AcquireLock (fun ltok -> checkFileInProjectCache.TryGet(ltok, (filename,source.GetHashCode(),options)))

match cachedResults with
// | Some (parseResults, checkResults, _, _) when builder.AreCheckResultsBeforeFileInProjectReady(filename) ->
Expand Down Expand Up @@ -2593,7 +2595,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
let rec loop() =
async {
// results may appear while we were waiting for the lock, let's recheck if it's the case
let cachedResults = bc.GetCachedCheckFileResult(builder, fileName, source, options)
let cachedResults = bc.GetCachedCheckFileResult(builder, fileName, source.GetHashCode(), options)

match cachedResults with
| Some (_, checkResults) -> return FSharpCheckFileAnswer.Succeeded checkResults
Expand Down Expand Up @@ -2695,7 +2697,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
}

/// Parses and checks the source file and returns untyped AST and check results.
member bc.ParseAndCheckFileInProject (filename:string, fileVersion, source, options:FSharpProjectOptions, textSnapshotInfo, userOpName) =
member bc.ParseAndCheckFileInProject (filename:string, fileVersion, source: string, options:FSharpProjectOptions, textSnapshotInfo, userOpName) =
let execWithReactorAsync action = reactor.EnqueueAndAwaitOpAsync(userOpName, "ParseAndCheckFileInProject", filename, action)
async {
try
Expand All @@ -2716,7 +2718,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
return (parseResults, FSharpCheckFileAnswer.Aborted)

| Some builder ->
let cachedResults = bc.GetCachedCheckFileResult(builder, filename, source, options)
let cachedResults = bc.GetCachedCheckFileResult(builder, filename, source.GetHashCode(), options)

match cachedResults with
| Some (parseResults, checkResults) ->
Expand Down Expand Up @@ -2782,7 +2784,7 @@ type BackgroundCompiler(legacyReferenceResolver, projectCacheSize, keepAssemblyC
match source with
| Some sourceText ->
parseCacheLock.AcquireLock (fun ltok ->
match checkFileInProjectCache.TryGet(ltok,(filename,sourceText,options)) with
match checkFileInProjectCache.TryGet(ltok,(filename,sourceText.GetHashCode(),options)) with
| Some (a,b,c,_) -> Some (a,b,c)
| None -> parseCacheLock.AcquireLock (fun ltok -> checkFileInProjectCachePossiblyStale.TryGet(ltok,(filename,options))))
| None -> parseCacheLock.AcquireLock (fun ltok -> checkFileInProjectCachePossiblyStale.TryGet(ltok,(filename,options)))
Expand Down Expand Up @@ -3018,14 +3020,15 @@ type FSharpChecker(legacyReferenceResolver, projectCacheSize, keepAssemblyConten

member ic.ReferenceResolver = legacyReferenceResolver

member ic.MatchBraces(filename, source, options: FSharpParsingOptions, ?userOpName: string) =
member ic.MatchBraces(filename, source: string, options: FSharpParsingOptions, ?userOpName: string) =
let userOpName = defaultArg userOpName "Unknown"
let sourceHash = source.GetHashCode()
async {
match braceMatchCache.TryGet(AssumeAnyCallerThreadWithoutEvidence(), (filename, source, options)) with
match braceMatchCache.TryGet(AssumeAnyCallerThreadWithoutEvidence(), (filename, sourceHash, options)) with
| Some res -> return res
| None ->
let res = Parser.matchBraces(source, filename, options, userOpName)
braceMatchCache.Set(AssumeAnyCallerThreadWithoutEvidence(), (filename, source, options), res)
braceMatchCache.Set(AssumeAnyCallerThreadWithoutEvidence(), (filename, sourceHash, options), res)
return res
}

Expand All @@ -3044,7 +3047,6 @@ type FSharpChecker(legacyReferenceResolver, projectCacheSize, keepAssemblyConten
ic.CheckMaxMemoryReached()
backgroundCompiler.ParseFile(filename, source, options, userOpName)


member ic.ParseFileInProject(filename, source, options, ?userOpName: string) =
let userOpName = defaultArg userOpName "Unknown"
let parsingOptions, _ = ic.GetParsingOptionsFromProjectOptions(options)
Expand All @@ -3059,7 +3061,7 @@ type FSharpChecker(legacyReferenceResolver, projectCacheSize, keepAssemblyConten
backgroundCompiler.GetBackgroundCheckResultsForFileInProject(filename,options, userOpName)

/// Try to get recent approximate type check results for a file.
member ic.TryGetRecentCheckResultsForFile(filename: string, options:FSharpProjectOptions, ?source, ?userOpName: string) =
member ic.TryGetRecentCheckResultsForFile(filename: string, options:FSharpProjectOptions, ?source: string, ?userOpName: string) =
let userOpName = defaultArg userOpName "Unknown"
backgroundCompiler.TryGetRecentCheckResultsForFile(filename,options,source, userOpName)

Expand Down