Skip to content

Commit d0f4180

Browse files
safesparrowsafesparrow
committed
cleanup
1 parent d43e0c3 commit d0f4180

File tree

4 files changed

+98
-90
lines changed

4 files changed

+98
-90
lines changed

‎tests/FSharp.Compiler.Service.Tests2/DepResolving.fs‎

Lines changed: 87 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -215,92 +215,95 @@ module internal AutomatedDependencyResolving =
215215

216216
let trie = buildTrie nodes
217217

218-
// Find dependencies for all files (can be in parallel)
218+
// Find dependencies for all files
219219
let graph =
220220
nodes
221221
|> Array.Parallel.map (fun node ->
222-
match node.ContainsModuleAbbreviations with
223-
| true -> node.Idx, allIndices
224-
| false ->
225-
let trie = cloneTrie trie
226-
227-
// Keep a list of visited nodes (ie. all reachable nodes and all their ancestors)
228-
let visited = emptyList<TrieNode>()
229-
230-
let markVisited (node : TrieNode) =
231-
if not node.Reachable then
232-
node.Reachable <- true
233-
visited.Add(node)
234-
235-
// Keep a list of reachable nodes (ie. ones that can be prefixes for later module/type references)
236-
let reachable = emptyList<TrieNode>()
237-
238-
let markReachable (node : TrieNode) =
239-
if not node.PotentialPrefix then
240-
node.PotentialPrefix <- true
241-
reachable.Add(node)
242-
markVisited node
243-
244-
// Mark root (no prefix) as reachable and visited
245-
markReachable trie
246-
247-
let rec extend (id : LongIdent) (node : TrieNode) =
248-
let rec extend (node : TrieNode) (id : LongIdent) =
249-
match id with
250-
// Reached end of the identifier - new reachable node
251-
| [] ->
252-
Some node
253-
// More segments exist
254-
| segment :: rest ->
255-
// Visit (not 'reach') the TrieNode
256-
markVisited node
257-
match node.Children.TryGetValue(segment.idText) with
258-
// A child for the segment exists - continue there
259-
| true, child ->
260-
extend child rest
261-
// A child for the segment doesn't exist - stop, since we don't care about the non-existent part of the Trie
262-
| false, _ ->
263-
None
264-
extend node id
265-
266-
// Process module refs in order, marking more and more TrieNodes as reachable
267-
let processRef (id : LongIdent) =
268-
let newReachables =
269-
// Start at every reachable node,
270-
reachable
271-
// extend a reachable node by 'id', but without creating new nodes, mark all seen nodes as visited and the final one as reachable
272-
|> Seq.choose (extend id)
273-
|> Seq.toArray
274-
newReachables
275-
|> Array.iter markReachable
276-
277-
// Add top-level module/namespaces as the first reference (possibly not necessary as maybe already in the list)
278-
// TODO When multiple top-level namespaces exist, we should check that it's OK to add all of them at the start (out of order).
279-
// Later on we might want to preserve the order by returning the top-level namespaces during AST visiting
280-
let moduleRefs =
281-
Array.append node.Tops node.ModuleRefs
282-
283-
// Process all refs
284-
moduleRefs
285-
|> Array.iter processRef
286-
287-
// Collect files from all visited TrieNodes
288-
let reachableItems =
289-
visited
290-
|> Seq.collect (fun node -> node.GraphNodes)
291-
|> Seq.toArray
292-
293-
// Return the node and its dependencies
294222
let deps =
295-
reachableItems
223+
// Assume that a file with module abbreviations can depend on anything
224+
match node.ContainsModuleAbbreviations with
225+
| true -> allIndices
226+
| false ->
227+
// Clone the original Trie as we're going to mutate the copy
228+
let trie = cloneTrie trie
229+
230+
// Keep a list of reachable nodes (ie. potential prefixes and their ancestors)
231+
let reachable = emptyList<TrieNode>()
232+
let markReachable (node : TrieNode) =
233+
if not node.Reachable then
234+
node.Reachable <- true
235+
reachable.Add(node)
236+
237+
// Keep a list of potential prefixes
238+
let potentialPrefixes = emptyList<TrieNode>()
239+
let markPotentialPrefix (node : TrieNode) =
240+
if not node.PotentialPrefix then
241+
node.PotentialPrefix <- true
242+
potentialPrefixes.Add(node)
243+
// Every potential prefix is reachable
244+
markReachable node
245+
246+
// Mark root (empty prefix) as a potential prefix
247+
markPotentialPrefix trie
248+
249+
/// <summary>
250+
/// Walk down from 'node' using 'id' as the path.
251+
/// Mark all visited nodes as reachable, and the final node as a potential prefix.
252+
/// Short-circuit when a leaf is reached.
253+
/// </summary>
254+
/// <remarks>
255+
/// When the path leads outside the Trie, the Trie is not extended and no node is marked as a potential prefix.
256+
/// This is just a performance optimisation - all the files are linked to already existing nodes, so there is no need to create and visit deeper nodes.
257+
/// </remarks>
258+
let rec walkDownAndMark (id : LongIdent) (node : TrieNode) =
259+
match id with
260+
// Reached end of the identifier - new reachable node
261+
| [] ->
262+
markPotentialPrefix node
263+
// More segments exist
264+
| segment :: rest ->
265+
// Visit (not 'reach') the TrieNode
266+
markReachable node
267+
match node.Children.TryGetValue(segment.idText) with
268+
// A child for the segment exists - continue there
269+
| true, child ->
270+
walkDownAndMark rest child
271+
// A child for the segment doesn't exist - stop, since we don't care about the non-existent part of the Trie
272+
| false, _ ->
273+
()
274+
275+
let processRef (id : LongIdent) =
276+
// Start at every potential prefix,
277+
List<_>(potentialPrefixes) // Copy the list for iteration as the original is going to be extended.
278+
// Extend potential prefixes with this 'id'
279+
|> Seq.iter (walkDownAndMark id)
280+
281+
// Add top-level module/namespaces as the first reference (possibly not necessary as maybe already in the list)
282+
// TODO When multiple top-level namespaces exist, we should check that it's OK to add all of them at the start (out of order).
283+
// Later on we might want to preserve the order by returning the top-level namespaces interleaved with module refs
284+
let moduleRefs =
285+
Array.append node.Tops node.ModuleRefs
286+
287+
// Process module refs in order, marking more and more TrieNodes as reachable and potential prefixes
288+
moduleRefs
289+
|> Array.iter processRef
290+
291+
// Collect files from all reachable TrieNodes
292+
let deps =
293+
reachable
294+
|> Seq.collect (fun node -> node.GraphNodes)
295+
|> Seq.map (fun n -> n.Idx)
296+
// Assume that this file depends on all files that have any module abbreviations
297+
// TODO Handle module abbreviations in a better way
298+
|> Seq.append filesWithModuleAbbreviations
299+
|> Seq.toArray
300+
301+
deps
296302
// We know a file can't depend on a file further down in the project definition (or on itself)
297-
|> Seq.filter (fun n -> n.Idx < node.Idx)
298-
|> Seq.map (fun n -> n.Idx)
299-
|> Seq.toArray
303+
|> Array.filter (fun depIdx -> depIdx < node.Idx)
300304

301-
let finalDeps = Array.append deps filesWithModuleAbbreviations
302-
303-
node.Idx, finalDeps
305+
// Return the node and its dependencies
306+
node.Idx, deps
304307
)
305308
|> dict
306309

@@ -312,6 +315,9 @@ module internal AutomatedDependencyResolving =
312315
log "Done"
313316
res
314317

318+
/// <summary>
319+
/// Calculate and print some stats about the expected parallelism factor of a dependency graph
320+
/// </summary>
315321
let analyseEfficiency (result : DepsResult) : unit =
316322
let totalFileSize =
317323
result.Files
@@ -339,7 +345,6 @@ let analyseEfficiency (result : DepsResult) : unit =
339345
| d -> d |> Array.map depthDfs |> Array.max
340346
let depth = int64(file.CodeSize) + deepestChild
341347
depths[idx] <- depth
342-
printfn $"Depth[{idx}, {file.Name}] <- {depth}"
343348
depth
344349
| depth ->
345350
// Already visited
@@ -351,4 +356,4 @@ let analyseEfficiency (result : DepsResult) : unit =
351356
|> Array.map (fun f -> depthDfs f.Idx)
352357
|> Array.max
353358

354-
printfn $"Total file size: {totalFileSize}. Max depth: {maxDepth}. Max Depth/Size = {maxDepth / totalFileSize}"
359+
printfn $"Total file size: {totalFileSize}. Max depth: {maxDepth}. Max Depth/Size = %.2f{double(maxDepth) / double(totalFileSize)}"

‎tests/FSharp.Compiler.Service.Tests2/RunCompiler.fs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
module FSharp.Compiler.Service.Tests.RunCompiler
22

3-
open FSharp.Build
3+
open NUnit.Framework
44

5+
[<Test>]
56
let runCompiler () =
67
let args =
78
System.IO.File.ReadAllLines(@"C:\projekty\fsharp\heuristic\tests\FSharp.Compiler.Service.Tests2\args.txt") |> Array.skip 1
8-
FSharp.Compiler.CommandLineMain.main args
9+
FSharp.Compiler.CommandLineMain.main args |> ignore

‎tests/FSharp.Compiler.Service.Tests2/TestASTVisit.fs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,8 @@ let x = 3
7777
printfn $"A refs: %+A{visitedA}"
7878
()
7979

80-
8180
[<Test>]
82-
let ``Test big`` () =
81+
let ``Test big.fs`` () =
8382
let code = System.IO.File.ReadAllText("Big.fs")
8483
let parsedA = getParseResults code
8584
let visitedA = extractModuleRefs parsedA

‎tests/FSharp.Compiler.Service.Tests2/TestDepResolving.fs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ let TestHardcodedFiles() =
9494
printfn "Detected file dependencies:"
9595
graph.Graph
9696
|> Seq.iter (fun (KeyValue(idx, deps)) -> printfn $"{graph.Files[idx].Name} -> %+A{deps |> Array.map(fun d -> graph.Files[d].Name)}")
97+
98+
analyseEfficiency graph
9799

98100
let private parseProjectAndGetSourceFiles (projectFile : string) =
99101
log "building project"
@@ -127,9 +129,10 @@ let TestProject (projectFile : string) =
127129
let totalDeps = graph.Graph |> Seq.sumBy (fun (KeyValue(idx, deps)) -> deps.Length)
128130
let maxPossibleDeps = (N * (N-1)) / 2
129131

130-
let graph = graph.Graph |> Seq.map (fun (KeyValue(idx, deps)) -> graph.Files[idx].Name, deps |> Array.map (fun d -> graph.Files[d].Name)) |> dict
131-
let json = JsonConvert.SerializeObject(graph, Formatting.Indented)
132+
let graphJson = graph.Graph |> Seq.map (fun (KeyValue(idx, deps)) -> graph.Files[idx].Name, deps |> Array.map (fun d -> graph.Files[d].Name)) |> dict
133+
let json = JsonConvert.SerializeObject(graphJson, Formatting.Indented)
132134
System.IO.File.WriteAllText("deps_graph.json", json)
133135

134-
printfn $"Analysed {N} files, detected {totalDeps}/{maxPossibleDeps} file dependencies:"
135-
printfn "Wrote graph as json in deps_graph.json"
136+
printfn $"Analysed {N} files, detected {totalDeps}/{maxPossibleDeps} file dependencies."
137+
printfn "Wrote graph as json in deps_graph.json"
138+
analyseEfficiency graph

0 commit comments

Comments
 (0)