diff --git a/src/app/FakeLib/TargetHelper.fs b/src/app/FakeLib/TargetHelper.fs index 465f795aa28..e11ef63356b 100644 --- a/src/app/FakeLib/TargetHelper.fs +++ b/src/app/FakeLib/TargetHelper.fs @@ -24,6 +24,12 @@ type private DependencyType = | Hard = 1 | Soft = 2 +type private DependencyLevel = + { + level:int; + dependants: string list; + } + /// [omit] let mutable PrintStackTraceOnError = false @@ -63,6 +69,8 @@ let reset() = ExecutedTargetTimes.Clear() FinalTargets.Clear() +let mutable CurrentTargetOrder = [] + /// Returns a list with all target names. let getAllTargetsNames() = TargetDict |> Seq.map (fun t -> t.Key) |> Seq.toList @@ -349,14 +357,16 @@ let private visitDependencies fVisit targetName = let visit fGetDependencies fVisit targetName = let visited = new HashSet<_>() let ordered = new List<_>() - let rec visitDependenciesAux level (depType,targetName) = + let rec visitDependenciesAux level (dependentTarget:option>) (depType,targetName) = let target = getTarget targetName let isVisited = visited.Contains targetName visited.Add targetName |> ignore - fVisit (target, depType, level, isVisited) - (fGetDependencies target) |> Seq.iter (visitDependenciesAux (level + 1)) + fVisit (dependentTarget, target, depType, level, isVisited) + + (fGetDependencies target) |> Seq.iter (visitDependenciesAux (level + 1) (Some target)) + if not isVisited then ordered.Add targetName - visitDependenciesAux 0 (DependencyType.Hard, targetName) + visitDependenciesAux 0 None (DependencyType.Hard, targetName) visited, ordered // First pass is to accumulate targets in (hard) dependency graph @@ -382,7 +392,7 @@ let PrintDependencyGraph verbose target = | true,target -> logfn "%sDependencyGraph for Target %s:" (if verbose then String.Empty else "Shortened ") target.Name - let logDependency ((t: TargetTemplate), depType, level, isVisited) = + let logDependency (_, (t: TargetTemplate), depType, level, isVisited) = if verbose || not isVisited then let indent = (String(' ', level * 3)) if depType = DependencyType.Soft then @@ -394,7 +404,11 @@ let PrintDependencyGraph verbose target = log "" log "The resulting target order is:" - Seq.iter (logfn " - %s") ordered + CurrentTargetOrder + |> List.iteri (fun index x -> + if (environVarOrDefault "parallel-jobs" "1" |> int > 1) then + logfn "Group - %d" (index + 1) + Seq.iter (logfn " - %s") x) /// Writes a summary of errors reported during build. let WriteErrors () = @@ -458,29 +472,63 @@ let determineBuildOrder (target : string) = let t = getTarget target - let targetLevels = new Dictionary<_,_>() - let addTargetLevel ((target: TargetTemplate), _, level, _ ) = - match targetLevels.TryGetValue target.Name with - | true, mapLevel when mapLevel >= level -> () - | _ -> targetLevels.[target.Name] <- level + let targetLevels = new Dictionary() + + let appendDepentantOption (currentList:string list) (dependantTarget:option>) = + match dependantTarget with + | None -> currentList + | Some x -> List.append currentList [x.Name] |> List.distinct + + let rec SetTargetLevel newLevel target = + match targetLevels.TryGetValue target with + | true, exDependencyLevel -> + if exDependencyLevel.level < newLevel then + exDependencyLevel.dependants |> List.iter (fun x -> SetTargetLevel (newLevel - 1) x) + if exDependencyLevel.dependants.Length > 0 then + targetLevels.[target] <- {level = newLevel; dependants = exDependencyLevel.dependants} + | _ -> () + + let addTargetLevel ((dependantTarget:option>), (target: TargetTemplate), _, level, _ ) = + let (|LevelIncreaseWithDependantTarget|_|) = function + | (true, exDependencyLevel), Some dt when exDependencyLevel.level > level -> Some (exDependencyLevel, dt) + | _ -> None + + let (|LevelIncreaseWithNoDependantTarget|_|) = function + | (true, exDependencyLevel), None when exDependencyLevel.level > level -> Some (exDependencyLevel) + | _ -> None + + let (|LevelDecrease|_|) = function + | (true, exDependencyLevel), _ when exDependencyLevel.level < level -> Some (exDependencyLevel) + | _ -> None + + let (|NewTarget|_|) = function + | (false, _), _ -> Some () + | _ -> None + + match targetLevels.TryGetValue target.Name, dependantTarget with + | LevelIncreaseWithDependantTarget (exDependencyLevel, dt) -> + SetTargetLevel (exDependencyLevel.level - 1) dt.Name + targetLevels.[target.Name] <- {level = exDependencyLevel.level; dependants = (appendDepentantOption exDependencyLevel.dependants dependantTarget)} + | LevelIncreaseWithNoDependantTarget (exDependencyLevel) -> + targetLevels.[target.Name] <- {level = exDependencyLevel.level; dependants = (appendDepentantOption exDependencyLevel.dependants dependantTarget)} + | LevelDecrease (exDependencyLevel) -> + exDependencyLevel.dependants |> List.iter (fun x -> SetTargetLevel (level - 1) x) + targetLevels.[target.Name] <- {level = level; dependants = (appendDepentantOption exDependencyLevel.dependants dependantTarget)} + | NewTarget -> + targetLevels.[target.Name] <- {level = level; dependants=(appendDepentantOption [] dependantTarget)} + | _ -> () let visited, ordered = visitDependencies addTargetLevel target // the results are grouped by their level, sorted descending (by level) and - // finally grouped together in a list[]> - let result = - targetLevels - |> Seq.map (fun pair -> pair.Key, pair.Value) - |> Seq.groupBy snd - |> Seq.sortBy (fun (l,_) -> -l) - |> Seq.map snd - |> Seq.map (fun v -> v |> Seq.map fst |> Seq.distinct |> Seq.map getTarget |> Seq.toArray) - |> Seq.toList - - // Note that this build order cannot be considered "optimal" - // since it may introduce order where actually no dependencies - // exist. However it yields a "good" execution order in practice. - result + // finally grouped together in a list[] + targetLevels + |> Seq.map (fun pair -> pair.Key, pair.Value.level) + |> Seq.groupBy snd + |> Seq.sortBy (fun (l,_) -> -l) + |> Seq.map snd + |> Seq.map (fun v -> v |> Seq.map fst |> Seq.distinct |> Seq.map getTarget |> Seq.toArray) + |> Seq.toList /// Runs a single target without its dependencies let runSingleTarget (target : TargetTemplate) = @@ -503,8 +551,6 @@ let runTargetsParallel (count : int) (targets : Target[]) = .ToArray() |> ignore -let mutable CurrentTargetOrder = [] - /// Runs a target and its dependencies. let run targetName = if doesTargetMeanListTargets targetName then listTargets() else @@ -537,20 +583,23 @@ let run targetName = order |> List.map (fun targets -> targets |> Array.map (fun t -> t.Name) |> Array.toList) + PrintDependencyGraph false targetName + // run every level in parallel for par in order do runTargetsParallel parallelJobs par else // single threaded build. - PrintDependencyGraph false targetName - + // Note: we could use the ordering resulting from flattening the result of determineBuildOrder // for a single threaded build (thereby centralizing the algorithm for build order), but that // ordering is inconsistent with earlier versions of FAKE (and PrintDependencyGraph). let _, ordered = visitDependencies ignore targetName CurrentTargetOrder <- ordered |> Seq.map (fun t -> [t]) |> Seq.toList + PrintDependencyGraph false targetName + runTargets (ordered |> Seq.map getTarget |> Seq.toArray) finally diff --git a/src/test/FsCheck.Fake/TestParallelBuildOrder.fs b/src/test/FsCheck.Fake/TestParallelBuildOrder.fs index e4528773215..c6bea5decc0 100644 --- a/src/test/FsCheck.Fake/TestParallelBuildOrder.fs +++ b/src/test/FsCheck.Fake/TestParallelBuildOrder.fs @@ -79,65 +79,208 @@ let ``Independent targets are parallel``() = () + +[] +let ``Issue #1395 Example``() = + TargetDict.Clear() + Target "T1" DoNothing + Target "T2.1" DoNothing + Target "T2.2" DoNothing + Target "T2.3" DoNothing + Target "T3" DoNothing + Target "T4" DoNothing + + // create a graph + "T1" ==> "T2.1" ==> "T2.2" ==> "T2.3" |> ignore + "T1" ==> "T3" |> ignore + "T2.3" ==> "T4" |> ignore + "T3" ==> "T4" |> ignore + + let order = determineBuildOrder "T4" + validateBuildOrder order "T4" + + match order with + | [[|Target "T1"|];TargetSet ["T2.1"; "T3"];[|Target "T2.2"|];[|Target "T2.3"|];[|Target "T4"|]] -> + // as expected + () + + | _ -> + failwithf "unexpected order: %A" order [] -let ``Ordering maintains dependencies``() = - let r = Random() +let ``Diamonds are resolved correctly``() = + TargetDict.Clear() + Target "a" DoNothing + Target "b" DoNothing + Target "c" DoNothing + Target "d" DoNothing - for iter in 1..10 do - TargetDict.Clear() - Target "final" DoNothing + // create graph + "a" ==> "b" ==> "d" |> ignore + "a" ==> "c" ==> "d" |> ignore - let targetCount = r.Next 30 + 10 - let dependencyCount = r.Next(3 * targetCount) + let order = determineBuildOrder "d" + validateBuildOrder order "d" - // define some targets and introduce a dependency - // to some final target. - for c in 0..targetCount-1 do - Target (string c) DoNothing - string c ==> "final" |> ignore + match order with + | [[|Target "a"|];TargetSet ["b"; "c"];[|Target "d"|]] -> + // as expected + () - // add a number of dependencies between two - // random targets. By adding dependencies from - // the lower index to the higher one we ensure that - // the resulting graph remains acyclic - for i in 0..dependencyCount-1 do - let a = r.Next targetCount - let b = r.Next targetCount + | _ -> + failwithf "unexpected order: %A" order - if a <> b then - // determine l(ow) and h(igh) and add the dependency - let l,h = if a < b then a,b else b, a - string l ==> string h |> ignore +[] +let ``Initial Targets Can Run Concurrently``() = + TargetDict.Clear() + Target "a" DoNothing + Target "b" DoNothing + Target "c1" DoNothing + Target "c2" DoNothing + Target "d" DoNothing + // create graph + "a" ==> "b" ==> "d" |> ignore + "c1" ==> "c2" ==> "d" |> ignore + let order = determineBuildOrder "d" + validateBuildOrder order "d" + + match order with + | [TargetSet ["a"; "c1"];TargetSet ["b"; "c2"];[|Target "d"|]] -> + // as expected + () - let order = determineBuildOrder "final" - validateBuildOrder order "final" + | _ -> + failwithf "unexpected order: %A" order [] -let ``Diamonds are resolved correctly``() = +let ``Spurs run as early as possible``() = TargetDict.Clear() Target "a" DoNothing Target "b" DoNothing - Target "c" DoNothing + Target "c1" DoNothing + Target "c2" DoNothing Target "d" DoNothing - // create a diamond graph + // create graph "a" ==> "b" ==> "d" |> ignore - "a" ==> "c" ==> "d" |> ignore + "a" ==> "c1" ==> "c2" ==> "d" |> ignore let order = determineBuildOrder "d" validateBuildOrder order "d" match order with - | [[|Target "a"|];TargetSet ["b"; "c"];[|Target "d"|]] -> + | [[|Target "a"|];TargetSet ["b"; "c1"];[|Target "c2"|];[|Target "d"|]] -> // as expected () | _ -> failwithf "unexpected order: %A" order +[] +let ``Spurs run as early as possible 3 and 2 length``() = + TargetDict.Clear() + Target "a" DoNothing + Target "b1" DoNothing + Target "b2" DoNothing + Target "c1" DoNothing + Target "c2" DoNothing + Target "c3" DoNothing + Target "d" DoNothing + + // create graph + "a" ==> "b1" ==> "b2" ==> "d" |> ignore + "a" ==> "c1" ==> "c2" ==> "c3" ==> "d" |> ignore + + let order = determineBuildOrder "d" + validateBuildOrder order "d" + + match order with + | [[|Target "a"|];TargetSet ["b1"; "c1"];TargetSet ["b2"; "c2"];[|Target "c3"|];[|Target "d"|]] -> + // as expected + () + + | _ -> + failwithf "unexpected order: %A" order + +[] +let ``Spurs run as early as possible (reverse definition order)``() = + TargetDict.Clear() + Target "a" DoNothing + Target "b" DoNothing + Target "c1" DoNothing + Target "c2" DoNothing + Target "d" DoNothing + + // create graph + "a" ==> "c1" ==> "c2" ==> "d" |> ignore + "a" ==> "b" ==> "d" |> ignore + + let order = determineBuildOrder "d" + validateBuildOrder order "d" + + match order with + | [[|Target "a"|];TargetSet ["b"; "c1"];[|Target "c2"|];[|Target "d"|]] -> + // as expected + () + + | _ -> + failwithf "unexpected order: %A" order + +[] +let ``Spurs run as early as possible split on longer spur``() = + TargetDict.Clear() + Target "a" DoNothing + Target "b" DoNothing + Target "c1" DoNothing + Target "c21" DoNothing + Target "c22" DoNothing + Target "d" DoNothing + + // create graph + "a" ==> "b" ==> "d" |> ignore + "a" ==> "c1" ==> "c21" ==> "d" |> ignore + "a" ==> "c1" ==> "c22" ==> "d" |> ignore + + let order = determineBuildOrder "d" + validateBuildOrder order "d" + + match order with + | [[|Target "a"|];TargetSet ["b"; "c1"];TargetSet ["c21"; "c22"];[|Target "d"|]] -> + // as expected + () + + | _ -> + failwithf "unexpected order: %A" order + +[] +let ``3 way Spurs run as early as possible``() = + TargetDict.Clear() + Target "a" DoNothing + Target "b" DoNothing + Target "c1" DoNothing + Target "c2" DoNothing + Target "d1" DoNothing + Target "d2" DoNothing + Target "d3" DoNothing + Target "e" DoNothing + + // create graph + "a" ==> "b" ==> "e" |> ignore + "a" ==> "c1" ==> "c2" ==> "e" |> ignore + "a" ==> "d1" ==> "d2" ==> "d3" ==> "e" |> ignore + + let order = determineBuildOrder "e" + validateBuildOrder order "e" + + match order with + | [[|Target "a"|];TargetSet ["b"; "c1"; "d1"];TargetSet ["c2"; "d2"];[|Target "d3"|];[|Target "e"|]] -> + // as expected + () + + | _ -> + failwithf "unexpected order: %A" order [] let ``Soft dependencies are respected when dependees are present``() = @@ -172,8 +315,6 @@ let ``Soft dependencies are respected when dependees are present``() = failwithf "unexpected order: %A" order () - - [] let ``Soft dependencies are ignored when dependees are not present``() = TargetDict.Clear()