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

[Ready for Review] Slightly improve performance #2299

Merged
merged 7 commits into from
May 2, 2017
+218 −136
Merged

[Ready for Review] Slightly improve performance #2299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ff81fa0
paket: performance by cloudRoutine and 0x53A
0x53A Apr 28, 2017
b7aca77
clean up Paket.fsproj: remove duplicated debugger options
0x53A Apr 28, 2017
7369948
don't compare tuples by reference identity
0x53A Apr 28, 2017
3347802
disable test "#1174 Should find Ninject error" again because it fails…
0x53A Apr 28, 2017
39e23a9
make findMatchingStep faster and slightly more readable
0x53A Apr 30, 2017
f5deb48
use slightly faster tryExtractOne, as demonstrated by benchmark (stil…
0x53A Apr 30, 2017
505c58c
performance: don't recalculate array on each iteration in fuseConflicts
0x53A Apr 30, 2017
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
10 changes: 10 additions & 0 deletions integrationtests/Paket.IntegrationTests/ResolverSkipsConflictsFastSpecs.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ let ``#1166 Should resolve Nancy without timeout``() =
lockFile.Groups.[Constants.MainDependencyGroup].Resolution.[PackageName "Nancy"].Version
|> shouldBeGreaterThan (SemVer.Parse "1.1")

[<Test>]
let ``#2289 Paket 4.x install command takes hours to complete``() =
let lockFile = install "i002289-resolve-nunit-timeout"
let nunitVersion =
lockFile.Groups.[Constants.MainDependencyGroup].Resolution.[PackageName "NUnit"].Version
nunitVersion
|> shouldBeGreaterThan (SemVer.Parse "2.0")
nunitVersion
|> shouldBeSmallerThan (SemVer.Parse "3.0")

[<Test>]
[<Ignore("fails with SO, skipping until works")>]
let ``#1174 Should find Ninject error``() =
Expand Down
52 changes: 52 additions & 0 deletions integrationtests/scenarios/i002289-resolve-nunit-timeout/before/paket.dependencies
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
redirects: on
framework: net46

source https://api.nuget.org/v3/index.json

nuget Akka.Persistence.SqlServer
nuget SQLProvider
nuget Microsoft.FSharpLu.Json
nuget Microsoft.Net.Http
nuget Microsoft.AspNet.WebApi.Client
nuget FSharp.Core redirects: force
nuget FSharp.Configuration
nuget FSharp.Formatting
nuget FSharp.Data
nuget FSharp.Data.TypeProviders
nuget Newtonsoft.Json
nuget FAKE
nuget SourceLink.Fake
nuget Akka
nuget Akka.FSharp
nuget Akka.Persistence
nuget Akka.Persistence.FSharp
nuget Akka.Persistence.Sql.Common
nuget Akka.Persistence.Query
nuget Akka.Persistence.Query.Sql
nuget Akka.Logger.Serilog
nuget Akka.Monitoring
nuget Akka.Monitoring.StatsD
nuget Akka.TestKit.NUnit
nuget Akka.TestKit.Xunit2
nuget Akka.Serialization.Wire
nuget FsPickler
nuget Serilog
nuget Serilog.Sinks.ElasticSearch
nuget Nest ~> 5
nuget RabbitMQ.Client
nuget SSH.NET
nuget Topshelf
nuget Topshelf.Serilog
nuget NUnit ~> 2
nuget NUnit.Runners
nuget NUnitTestAdapter
nuget TickSpec.NUnit content: none
nuget FsCheck
nuget FsCheck.Nunit
nuget System.Collections.Immutable
nuget Suave
nuget OctopusTools
nuget Suave.Swagger
nuget Quartz
nuget Akka.Quartz.Actor
nuget Oracle.ManagedDataAccess
11 changes: 11 additions & 0 deletions src/Paket.Core/Common/Utils.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,3 +999,14 @@ module Seq =
) |> fun (xs,ys) ->
List.rev xs :> seq<_>, List.rev ys :> seq<_>

[<RequireQualifiedAccess>]
module List =
// Try to find an element in a list.
// If found, return the element and the list WITHOUT the element.
// If not found, return None and the whole original list.
let tryExtractOne fn values =
match List.tryFindIndex fn values with
| Some i ->
let v = values.[i]
Some v, (values.[0 .. i - 1 ] @ values.[i + 1 .. ])
| None -> None, values
174 changes: 100 additions & 74 deletions src/Paket.Core/Dependencies/PackageResolver.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ let calcOpenRequirements (exploredPackage:ResolvedPackage,globalFrameworkRestric
Settings = { dependency.Settings with FrameworkRestrictions = newRestrictions } })
|> Set.filter (fun d ->
resolverStep.ClosedRequirements
|> Seq.exists (fun x ->
|> Set.exists (fun x ->
x.Name = d.Name &&
x.Settings.FrameworkRestrictions = d.Settings.FrameworkRestrictions &&
(x = d ||
Expand All @@ -281,7 +281,7 @@ let calcOpenRequirements (exploredPackage:ResolvedPackage,globalFrameworkRestric
|> not)
|> Set.filter (fun d ->
resolverStep.OpenRequirements
|> Seq.exists (fun x -> x.Name = d.Name && (x = d || x.VersionRequirement.Range.IsGlobalOverride) && x.Settings.FrameworkRestrictions = d.Settings.FrameworkRestrictions)
|> Set.exists (fun x -> x.Name = d.Name && (x = d || x.VersionRequirement.Range.IsGlobalOverride) && x.Settings.FrameworkRestrictions = d.Settings.FrameworkRestrictions)
|> not)
|> Set.union rest

Expand Down Expand Up @@ -390,7 +390,7 @@ let private explorePackageConfig getPackageDetailsF (pkgConfig:PackageConfig) =

type StackPack = {
ExploredPackages : Dictionary<PackageName*SemVerInfo,ResolvedPackage>
KnownConflicts : HashSet<Set<PackageRequirement> * ((SemVerInfo * PackageSource list) list * bool) option>
KnownConflicts : HashSet<HashSet<PackageRequirement> * ((SemVerInfo * PackageSource list) list * bool) option>
ConflictHistory : Dictionary<PackageName, int>
}

Expand Down Expand Up @@ -494,23 +494,26 @@ let private getCompatibleVersions
compatibleVersions, false, tryRelaxed


let private getConflicts (currentStep:ResolverStep) (currentRequirement:PackageRequirement) (knownConflicts:HashSet<_>) =
let private getConflicts (currentStep:ResolverStep) (currentRequirement:PackageRequirement) (knownConflicts:HashSet<HashSet<PackageRequirement> * ((SemVerInfo * PackageSource list) list * bool) option>) =

let allRequirements =
currentStep.OpenRequirements
|> Set.filter (fun r -> r.Graph |> List.contains currentRequirement |> not)
|> Set.union currentStep.ClosedRequirements
Set.toSeq currentStep.OpenRequirements
|> Seq.filter (fun r -> r.Graph |> List.contains currentRequirement |> not)
|> Seq.append currentStep.ClosedRequirements
|> HashSet

knownConflicts
|> Seq.map (fun (conflicts,selectedVersion) ->
match selectedVersion with
| None when Set.isSubset conflicts allRequirements -> conflicts
| None when conflicts.IsSubsetOf allRequirements -> conflicts
| Some(selectedVersion,_) ->
let n = (Seq.head conflicts).Name
match currentStep.FilteredVersions |> Map.tryFind n with
| Some(v,_) when v = selectedVersion && Set.isSubset conflicts allRequirements -> conflicts
| _ -> Set.empty
| _ -> Set.empty)
|> Set.unionMany
| Some(v,_) when v = selectedVersion && conflicts.IsSubsetOf allRequirements -> conflicts
| _ -> HashSet()
| _ -> HashSet())
|> Seq.collect id
|> HashSet


let private getCurrentRequirement packageFilter (openRequirements:Set<PackageRequirement>) (conflictHistory:Dictionary<_,_>) =
Expand Down Expand Up @@ -575,7 +578,7 @@ let private boostConflicts
match conflicts with
| c::_ ->
let selectedVersion = Map.tryFind c.Name filteredVersions
let key = conflicts |> Set.ofList,selectedVersion
let key = conflicts |> HashSet,selectedVersion
stackpack.KnownConflicts.Add key |> ignore

let reportThatResolverIsTakingLongerThanExpected =
Expand All @@ -597,25 +600,24 @@ let private boostConflicts


[<Struct>]
type private StepFlags (ready:bool,useUnlisted:bool,hasUnlisted:bool,forceBreak:bool,firstTrial:bool,unlistedSearch:bool) =
member __.Ready = ready
member __.UseUnlisted = useUnlisted
member __.HasUnlisted = hasUnlisted
member __.ForceBreak = forceBreak
member __.FirstTrial = firstTrial
member __.UnlistedSearch = unlistedSearch
member private self.Display
with get () =
sprintf
"[< FLAGS >]\n\
| Ready - %b\n\
| UseUnlisted - %b\n\
| HasUnlisted - %b\n\
| ForceBreak - %b\n\
| FirstTrial - %b\n\
| UnlistedSearch - %b\n"
ready useUnlisted hasUnlisted forceBreak firstTrial unlistedSearch
override self.ToString() = self.Display
type private StepFlags = {
Ready : bool
UseUnlisted : bool
HasUnlisted : bool
ForceBreak : bool
FirstTrial : bool
UnlistedSearch : bool
} with
override self.ToString () =
sprintf
"[< FLAGS >]\n\
| Ready - %b\n\
| UseUnlisted - %b\n\
| HasUnlisted - %b\n\
| ForceBreak - %b\n\
| FirstTrial - %b\n\
| UnlistedSearch - %b\n"
self.Ready self.UseUnlisted self.HasUnlisted self.ForceBreak self.FirstTrial self.UnlistedSearch

type private Stage =
| Step of currentConflict : (ConflictState * ResolverStep * PackageRequirement) * priorConflictSteps : (ConflictState * ResolverStep * PackageRequirement * seq<SemVerInfo * PackageSource list> * StepFlags) list
Expand Down Expand Up @@ -649,14 +651,33 @@ let Resolve (getVersionsF, getPackageDetailsF, groupName:GroupName, globalStrate

let rec step (stage:Stage) (stackpack:StackPack) compatibleVersions (flags:StepFlags) =

let inline fuseConflicts currentConflict priorConflictSteps =
match currentConflict, priorConflictSteps with
| currentConflict, (lastConflict,lastStep,lastRequirement,lastCompatibleVersions,lastFlags)::priorConflictSteps ->
let inline fuseConflicts currentConflict priorConflictSteps conflicts =
let findMatchingStep priorConflictSteps =
let currentNames =
conflicts
|> Seq.collect (fun c ->
let graphNameList =
c.Graph |> List.map (fun (pr:PackageRequirement) -> pr.Name)
c.Name :: graphNameList)
|> Seq.toArray
priorConflictSteps
|> List.tryExtractOne (fun (_,_,lastRequirement:PackageRequirement,_,_) ->
currentNames |> Array.contains lastRequirement.Name)

match findMatchingStep priorConflictSteps with
| None, [] -> currentConflict
| (Some head), priorConflictSteps ->
let (lastConflict, lastStep, lastRequirement, lastCompatibleVersions, lastFlags) = head
let continueConflict =
{ currentConflict with VersionsToExplore = lastConflict.VersionsToExplore }
step (Inner((continueConflict,lastStep,lastRequirement),priorConflictSteps)) stackpack lastCompatibleVersions lastFlags
| currentConflict, [] -> currentConflict

step (Inner((continueConflict,lastStep,lastRequirement), priorConflictSteps)) stackpack lastCompatibleVersions lastFlags
// could not find a specific package - go back one step
| None, head :: priorConflictSteps ->
let (lastConflict, lastStep, lastRequirement, lastCompatibleVersions, lastFlags) = head
let continueConflict =
{ currentConflict with VersionsToExplore = lastConflict.VersionsToExplore }
step (Inner((continueConflict,lastStep,lastRequirement), priorConflictSteps)) stackpack lastCompatibleVersions lastFlags

match stage with
| Step((currentConflict,currentStep,_currentRequirement), priorConflictSteps) ->
if Set.isEmpty currentStep.OpenRequirements then
Expand All @@ -678,10 +699,8 @@ let Resolve (getVersionsF, getPackageDetailsF, groupName:GroupName, globalStrate
&& not (conflicts |> Set.exists (fun r ->
r = lastRequirement
|| r.Graph |> List.contains lastRequirement)) ->
let flags =
StepFlags(flags.Ready,flags.UseUnlisted,flags.HasUnlisted,true,flags.FirstTrial,flags.UnlistedSearch)

step (Inner((continueConflict,lastStep,lastRequirement),priorConflictSteps)) stackpack lastCompatibleVersions lastFlags
step (Inner((continueConflict,lastStep,lastRequirement),priorConflictSteps)) stackpack lastCompatibleVersions { flags with ForceBreak = true }
| _ ->
step (Inner((continueConflict,lastStep,lastRequirement),priorConflictSteps)) stackpack lastCompatibleVersions lastFlags

Expand All @@ -703,15 +722,15 @@ let Resolve (getVersionsF, getPackageDetailsF, groupName:GroupName, globalStrate

let currentConflict =
let getVersionsF = getVersionsF currentRequirement.Sources ResolverStrategy.Max groupName
if Set.isEmpty conflicts then
if Seq.isEmpty conflicts then
{ currentConflict with
Status = Resolution.Conflict(currentStep,Set.empty,currentRequirement,getVersionsF)}
Status = Resolution.Conflict (currentStep,Set.empty,currentRequirement,getVersionsF)}
else
{ currentConflict with
Status = Resolution.Conflict(currentStep,conflicts,Seq.head conflicts,getVersionsF)}
Status = Resolution.Conflict (currentStep,set conflicts,Seq.head conflicts,getVersionsF)}

if not (Set.isEmpty conflicts) then
fuseConflicts currentConflict priorConflictSteps
if not (Seq.isEmpty conflicts) then
fuseConflicts currentConflict priorConflictSteps conflicts
else
let compatibleVersions,globalOverride,tryRelaxed =
getCompatibleVersions currentStep groupName currentRequirement getVersionsF
Expand All @@ -721,7 +740,7 @@ let Resolve (getVersionsF, getPackageDetailsF, groupName:GroupName, globalStrate

let currentConflict = {
currentConflict with
Conflicts = conflicts
Conflicts = set conflicts
TryRelaxed = tryRelaxed
GlobalOverride = globalOverride
}
Expand All @@ -730,21 +749,23 @@ let Resolve (getVersionsF, getPackageDetailsF, groupName:GroupName, globalStrate
boostConflicts currentStep.FilteredVersions currentRequirement stackpack currentConflict
else
currentConflict, stackpack
let flags =
StepFlags
( ready = false
, useUnlisted = false
, hasUnlisted = false
, forceBreak = flags.ForceBreak
, firstTrial = flags.FirstTrial
, unlistedSearch = false
)
let flags = {
flags with
Ready = false
UseUnlisted = false
HasUnlisted = false
UnlistedSearch = false
}
step (Outer ((conflictState,currentStep,currentRequirement),priorConflictSteps)) stackpack compatibleVersions flags
| Outer ((currentConflict,currentStep,currentRequirement), priorConflictSteps) ->
if flags.Ready then
fuseConflicts currentConflict priorConflictSteps
if flags.Ready then
fuseConflicts currentConflict priorConflictSteps (HashSet [ currentRequirement ])
else
let flags = StepFlags(flags.Ready,flags.UseUnlisted,flags.HasUnlisted,false,true,flags.UnlistedSearch)
let flags = {
flags with
ForceBreak = false
FirstTrial = true
}
let currentConflict = { currentConflict with VersionsToExplore = compatibleVersions }
step (Inner ((currentConflict,currentStep,currentRequirement), priorConflictSteps)) stackpack compatibleVersions flags

Expand All @@ -758,14 +779,21 @@ let Resolve (getVersionsF, getPackageDetailsF, groupName:GroupName, globalStrate
then
// if it's been determined that an unlisted package must be used, ready must be set to false
verbosefn "\nSearching for compatible unlisted package\n"
StepFlags(false,true,flags.HasUnlisted,flags.ForceBreak,flags.FirstTrial,true)
{ flags with
Ready = false
UseUnlisted = true
UnlistedSearch = true
}
else
StepFlags(true,flags.UseUnlisted,flags.HasUnlisted,flags.ForceBreak,flags.FirstTrial,false)
{ flags with
Ready = true
UnlistedSearch = true
}
step (Outer((currentConflict,currentStep,currentRequirement), priorConflictSteps)) stackpack compatibleVersions flags
else


let flags = StepFlags(flags.Ready,flags.UseUnlisted,flags.HasUnlisted,flags.ForceBreak,false,flags.UnlistedSearch)
let flags = { flags with FirstTrial = false }
let (version,sources) & versionToExplore = Seq.head currentConflict.VersionsToExplore

let currentConflict =
Expand All @@ -787,8 +815,7 @@ let Resolve (getVersionsF, getPackageDetailsF, groupName:GroupName, globalStrate

| stackpack, Some(alreadyExplored,exploredPackage) ->
let hasUnlisted = exploredPackage.Unlisted || flags.HasUnlisted
let flags =
StepFlags(flags.Ready,flags.UseUnlisted,hasUnlisted,flags.ForceBreak,flags.FirstTrial,flags.UnlistedSearch)
let flags = { flags with HasUnlisted = hasUnlisted }
Copy link
Member

Choose a reason for hiding this comment

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

let flags = { flags with HasUnlisted = exploredPackage.Unlisted || flags.HasUnlisted }


if exploredPackage.Unlisted && not flags.UseUnlisted then
if not alreadyExplored then
Expand Down Expand Up @@ -835,19 +862,18 @@ let Resolve (getVersionsF, getPackageDetailsF, groupName:GroupName, globalStrate

let stackpack = {
ExploredPackages = Dictionary<PackageName*SemVerInfo,ResolvedPackage>()
KnownConflicts = (HashSet() : HashSet<Set<PackageRequirement> * ((SemVerInfo * PackageSource list) list * bool) option>)
KnownConflicts = (HashSet() : HashSet<HashSet<PackageRequirement> * ((SemVerInfo * PackageSource list) list * bool) option>)
ConflictHistory = (Dictionary() : Dictionary<PackageName, int>)
}

let flags =
StepFlags
( ready = false
, useUnlisted = false
, hasUnlisted = false
, forceBreak = false
, firstTrial = true
, unlistedSearch = false
)
let flags = {
Ready = false
UseUnlisted = false
HasUnlisted = false
ForceBreak = false
FirstTrial = true
UnlistedSearch = false
}

match step (Step((currentConflict,startingStep,currentRequirement),[])) stackpack Seq.empty flags with
| { Status = Resolution.Conflict _ } as conflict ->
Expand Down
Loading