Skip to content

Commit

Permalink
Works by forcing full dep graph
Browse files Browse the repository at this point in the history
  • Loading branch information
safesparrow committed Nov 5, 2022
1 parent 9d29b49 commit 8900e78
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 10 deletions.
2 changes: 1 addition & 1 deletion tests/ParallelTypeCheckingTests/Code/DepResolving.fs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ module internal AutomatedDependencyResolving =
|> Array.filter (fun dep -> dep.Idx < node.File.Idx)
// TODO Temporary - bring this back
// Filter out deps onto .fs files that have backing .fsi files
// |> Array.filter (fun dep -> not dep.FsiBacked)
|> Array.filter (fun dep -> not dep.FsiBacked)
|> Array.distinct

// Return the node and its dependencies
Expand Down
55 changes: 53 additions & 2 deletions tests/ParallelTypeCheckingTests/Code/GraphProcessing.fs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type Node<'Item, 'State, 'Result> =
/// <param name="deps">Direct dependencies of a node</param>
/// <param name="transitiveDeps">Transitive dependencies of a node</param>
/// <param name="folder">A way to fold a single result into existing state</param>
let combineResults
let combineResultsOld
(emptyState : 'State)
(deps : Node<'Item, 'State, 'Result>[])
(transitiveDeps : Node<'Item, 'State, 'Result>[])
Expand Down Expand Up @@ -87,8 +87,58 @@ let combineResults
set.Add biggestDep.Info.Item |> ignore
set
let resultsToAdd =
transitiveDeps
transitiveDeps
|> Array.filter (fun dep -> included.Contains dep.Info.Item = false)
|> Array.distinctBy (fun dep -> dep.Info.Item)
|> Array.map (fun dep ->
dep.Result
|> orFail
|> snd
)
let state = Array.fold folder firstState resultsToAdd
state


// TODO Do we need to suppress some error logging if we
// TODO apply the same partial results multiple times?
// TODO Maybe we can enable logging only for the final fold
/// <summary>
/// Combine results of dependencies needed to type-check a 'higher' node in the graph
/// </summary>
/// <param name="deps">Direct dependencies of a node</param>
/// <param name="transitiveDeps">Transitive dependencies of a node</param>
/// <param name="folder">A way to fold a single result into existing state</param>
let combineResults
(emptyState : 'State)
(deps : Node<'Item, 'State, 'Result>[])
(transitiveDeps : Node<'Item, 'State, 'Result>[])
(folder : 'State -> 'Result -> 'State)
: 'State
=
match deps with
| [||] -> emptyState
| _ ->
let orFail value =
value
|> Option.defaultWith (fun () -> failwith "Unexpected lack of result")
let firstState = emptyState
// TODO Potential perf optimisation: Keep transDeps in a HashSet from the start,
// avoiding reconstructing the HashSet here

// Add single-file results of remaining transitive deps one-by-one using folder
// Note: Good to preserve order here so that folding happens in file order
let included =
let set = HashSet()
//set.Add biggestDep.Info.Item |> ignore
set
let resultsToAdd =
transitiveDeps
// Sort it by effectively file index.
// For some reason this is needed, otherwise gives 'missing namespace' and other errors when using the resulting state.
// Does this make sense? Should the results be foldable in any order?
|> Array.sortBy (fun d -> d.Info.Item)
|> Array.filter (fun dep -> included.Contains dep.Info.Item = false)
|> Array.distinctBy (fun dep -> dep.Info.Item)
|> Array.map (fun dep ->
dep.Result
|> orFail
Expand All @@ -97,6 +147,7 @@ let combineResults
let state = Array.fold folder firstState resultsToAdd
state


// TODO Could be replaced with a simpler recursive approach with memoised per-item results
let processGraph<'Item, 'State, 'Result, 'FinalFileResult when 'Item : equality and 'Item : comparison>
(graph : Graph<'Item>)
Expand Down
12 changes: 6 additions & 6 deletions tests/ParallelTypeCheckingTests/Code/ParallelTypeChecking.fs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ module internal Real =
)
|> Seq.append (fsiXMap |> Seq.map (fun (KeyValue(fsi, x)) -> x.File, [|fsi|]))
|> readOnlyDict
// let graph =
// {
// Files = Array.append graph.Files xFiles
// Graph = stuff |> Graph.fillEmptyNodes
// } : DepsResult
let graph =
{
Files = Array.append graph.Files xFiles
Graph = stuff |> Graph.fillEmptyNodes
} : DepsResult
graph.Graph |> Graph.print

let graphJson = graph.Graph |> Seq.map (fun (KeyValue(file, deps)) -> file.Name, deps |> Array.map (fun d -> d.Name)) |> dict
Expand Down Expand Up @@ -262,7 +262,7 @@ module internal Real =
folder
state
(fun it -> not <| it.Name.EndsWith(".fsix"))
10
1

partialResults |> Array.toList, tcState
)
Expand Down
2 changes: 1 addition & 1 deletion tests/ParallelTypeCheckingTests/Tests/FCS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@
--deterministic+
--simpleresolution
--nowarn:3384
--times
--nowarn:75
--extraoptimizationloops:1
--warnon:1182
Expand All @@ -205,6 +204,7 @@ $CODE_ROOT$\artifacts\obj\FSharp.Compiler.Service\Debug\netstandard2.0\buildprop
$CODE_ROOT$\artifacts\obj\FSharp.Compiler.Service\Debug\netstandard2.0\FSharp.Compiler.Service.InternalsVisibleTo.fs
$CODE_ROOT$\artifacts\obj\FSharp.Compiler.Service\Debug\netstandard2.0\.NETStandard,Version=v2.0.AssemblyAttributes.fs
$CODE_ROOT$\artifacts\obj\FSharp.Compiler.Service\Debug\netstandard2.0\FSharp.Compiler.Service.AssemblyInfo.fs
Utilities\Test.fs
Utilities\sformat.fsi
Utilities\sformat.fs
Utilities\sr.fsi
Expand Down

0 comments on commit 8900e78

Please sign in to comment.