Skip to content

Commit 8900e78

Browse files
safesparrowsafesparrow
committed
Works by forcing full dep graph
1 parent 9d29b49 commit 8900e78

4 files changed

Lines changed: 61 additions & 10 deletions

File tree

‎tests/ParallelTypeCheckingTests/Code/DepResolving.fs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ module internal AutomatedDependencyResolving =
257257
|> Array.filter (fun dep -> dep.Idx < node.File.Idx)
258258
// TODO Temporary - bring this back
259259
// Filter out deps onto .fs files that have backing .fsi files
260-
// |> Array.filter (fun dep -> not dep.FsiBacked)
260+
|> Array.filter (fun dep -> not dep.FsiBacked)
261261
|> Array.distinct
262262

263263
// Return the node and its dependencies

‎tests/ParallelTypeCheckingTests/Code/GraphProcessing.fs‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type Node<'Item, 'State, 'Result> =
4848
/// <param name="deps">Direct dependencies of a node</param>
4949
/// <param name="transitiveDeps">Transitive dependencies of a node</param>
5050
/// <param name="folder">A way to fold a single result into existing state</param>
51-
let combineResults
51+
let combineResultsOld
5252
(emptyState : 'State)
5353
(deps : Node<'Item, 'State, 'Result>[])
5454
(transitiveDeps : Node<'Item, 'State, 'Result>[])
@@ -87,8 +87,58 @@ let combineResults
8787
set.Add biggestDep.Info.Item |> ignore
8888
set
8989
let resultsToAdd =
90-
transitiveDeps
90+
transitiveDeps
91+
|> Array.filter (fun dep -> included.Contains dep.Info.Item = false)
92+
|> Array.distinctBy (fun dep -> dep.Info.Item)
93+
|> Array.map (fun dep ->
94+
dep.Result
95+
|> orFail
96+
|> snd
97+
)
98+
let state = Array.fold folder firstState resultsToAdd
99+
state
100+
101+
102+
// TODO Do we need to suppress some error logging if we
103+
// TODO apply the same partial results multiple times?
104+
// TODO Maybe we can enable logging only for the final fold
105+
/// <summary>
106+
/// Combine results of dependencies needed to type-check a 'higher' node in the graph
107+
/// </summary>
108+
/// <param name="deps">Direct dependencies of a node</param>
109+
/// <param name="transitiveDeps">Transitive dependencies of a node</param>
110+
/// <param name="folder">A way to fold a single result into existing state</param>
111+
let combineResults
112+
(emptyState : 'State)
113+
(deps : Node<'Item, 'State, 'Result>[])
114+
(transitiveDeps : Node<'Item, 'State, 'Result>[])
115+
(folder : 'State -> 'Result -> 'State)
116+
: 'State
117+
=
118+
match deps with
119+
| [||] -> emptyState
120+
| _ ->
121+
let orFail value =
122+
value
123+
|> Option.defaultWith (fun () -> failwith "Unexpected lack of result")
124+
let firstState = emptyState
125+
// TODO Potential perf optimisation: Keep transDeps in a HashSet from the start,
126+
// avoiding reconstructing the HashSet here
127+
128+
// Add single-file results of remaining transitive deps one-by-one using folder
129+
// Note: Good to preserve order here so that folding happens in file order
130+
let included =
131+
let set = HashSet()
132+
//set.Add biggestDep.Info.Item |> ignore
133+
set
134+
let resultsToAdd =
135+
transitiveDeps
136+
// Sort it by effectively file index.
137+
// For some reason this is needed, otherwise gives 'missing namespace' and other errors when using the resulting state.
138+
// Does this make sense? Should the results be foldable in any order?
139+
|> Array.sortBy (fun d -> d.Info.Item)
91140
|> Array.filter (fun dep -> included.Contains dep.Info.Item = false)
141+
|> Array.distinctBy (fun dep -> dep.Info.Item)
92142
|> Array.map (fun dep ->
93143
dep.Result
94144
|> orFail
@@ -97,6 +147,7 @@ let combineResults
97147
let state = Array.fold folder firstState resultsToAdd
98148
state
99149

150+
100151
// TODO Could be replaced with a simpler recursive approach with memoised per-item results
101152
let processGraph<'Item, 'State, 'Result, 'FinalFileResult when 'Item : equality and 'Item : comparison>
102153
(graph : Graph<'Item>)

‎tests/ParallelTypeCheckingTests/Code/ParallelTypeChecking.fs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,11 @@ module internal Real =
132132
)
133133
|> Seq.append (fsiXMap |> Seq.map (fun (KeyValue(fsi, x)) -> x.File, [|fsi|]))
134134
|> readOnlyDict
135-
// let graph =
136-
// {
137-
// Files = Array.append graph.Files xFiles
138-
// Graph = stuff |> Graph.fillEmptyNodes
139-
// } : DepsResult
135+
let graph =
136+
{
137+
Files = Array.append graph.Files xFiles
138+
Graph = stuff |> Graph.fillEmptyNodes
139+
} : DepsResult
140140
graph.Graph |> Graph.print
141141

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

267267
partialResults |> Array.toList, tcState
268268
)

‎tests/ParallelTypeCheckingTests/Tests/FCS.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@
191191
--deterministic+
192192
--simpleresolution
193193
--nowarn:3384
194-
--times
195194
--nowarn:75
196195
--extraoptimizationloops:1
197196
--warnon:1182
@@ -205,6 +204,7 @@ $CODE_ROOT$\artifacts\obj\FSharp.Compiler.Service\Debug\netstandard2.0\buildprop
205204
$CODE_ROOT$\artifacts\obj\FSharp.Compiler.Service\Debug\netstandard2.0\FSharp.Compiler.Service.InternalsVisibleTo.fs
206205
$CODE_ROOT$\artifacts\obj\FSharp.Compiler.Service\Debug\netstandard2.0\.NETStandard,Version=v2.0.AssemblyAttributes.fs
207206
$CODE_ROOT$\artifacts\obj\FSharp.Compiler.Service\Debug\netstandard2.0\FSharp.Compiler.Service.AssemblyInfo.fs
207+
Utilities\Test.fs
208208
Utilities\sformat.fsi
209209
Utilities\sformat.fs
210210
Utilities\sr.fsi

0 commit comments

Comments
 (0)