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.

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

Resolving dependencies takes too long #1524

Closed
Stift opened this Issue Mar 17, 2016 · 38 comments

Comments

Projects
None yet
3 participants
@Stift
Contributor

Stift commented Mar 17, 2016

We have around 90+ direct dependencies and like ~200 dependencies in total in our project and since v2.52.9 the resolving takes 'forever'. It seems for me, that the fix of #1520 has caused that problem. Before an paket install took like 7-10s and now we are waiting +15min at least (I had to stop this and did not measure the correct timing).

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

yeah looks like a massive perf issue. I will investigate
/cc @isaacabraham

Member

forki commented Mar 17, 2016

yeah looks like a massive perf issue. I will investigate
/cc @isaacabraham

@forki forki added the bug label Mar 17, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

I just reverted that change. Can just please check if perf is restored? Seems I need to investigate deeper

Member

forki commented Mar 17, 2016

I just reverted that change. Can just please check if perf is restored? Seems I need to investigate deeper

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

pushed 2.52.12 - I think I verschlimmbessert it in 2.52.11

Member

forki commented Mar 17, 2016

pushed 2.52.12 - I think I verschlimmbessert it in 2.52.11

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

Tested it with 2.52.12. and it's back to normal. Thx.

Contributor

Stift commented Mar 17, 2016

Tested it with 2.52.12. and it's back to normal. Thx.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

OK now I need to find a proper fix for #1520
@Stift I guess I need to push another version soon and ask you again if perf is still good ;-)

Member

forki commented Mar 17, 2016

OK now I need to find a proper fix for #1520
@Stift I guess I need to push another version soon and ask you again if perf is still good ;-)

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

Just do so. I'm glad to test it.
But today is St. Patricks Day and I'm off and out in a few hours:

Contributor

Stift commented Mar 17, 2016

Just do so. I'm glad to test it.
But today is St. Patricks Day and I'm off and out in a few hours:

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

2.52.13 will be online in 2min. Please give it a try

Member

forki commented Mar 17, 2016

2.52.13 will be online in 2min. Please give it a try

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

that will fit

Contributor

Stift commented Mar 17, 2016

that will fit

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

so it's online

Member

forki commented Mar 17, 2016

so it's online

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

I'm already testing it. I get some warnings that I cannot specifcally say what they are about:

Could not resolve package Npgsql:
 - Dependencies file requested: 2.2.5-custom
 - <customPackage1> 1.0.11044 requested: >= 0
 - <customPackage2> 1.0.11149 requested: >= 2.2.5
 - <customPackage3> 1.1.10087 requested: >= 2.0.0 < 3.0.0

Also some of the numbers do not fit to the package itself. Is something mixing up here?

Contributor

Stift commented Mar 17, 2016

I'm already testing it. I get some warnings that I cannot specifcally say what they are about:

Could not resolve package Npgsql:
 - Dependencies file requested: 2.2.5-custom
 - <customPackage1> 1.0.11044 requested: >= 0
 - <customPackage2> 1.0.11149 requested: >= 2.2.5
 - <customPackage3> 1.1.10087 requested: >= 2.0.0 < 3.0.0

Also some of the numbers do not fit to the package itself. Is something mixing up here?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

shit. soo I'm not done

Member

forki commented Mar 17, 2016

shit. soo I'm not done

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

Also after a second run of install downloads now all packages again and this again takes a while.
First run: 26s
Second run (downloading?): +5min

Contributor

Stift commented Mar 17, 2016

Also after a second run of install downloads now all packages again and this again takes a while.
First run: 26s
Second run (downloading?): +5min

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

sorry for that

Contributor

Stift commented Mar 17, 2016

sorry for that

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

ok this one is really driving me nuts.

I released another version. Can you give me your deps file by any chance?

Member

forki commented Mar 17, 2016

ok this one is really driving me nuts.

I released another version. Can you give me your deps file by any chance?

@forki forki closed this in 73715db Mar 17, 2016

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

I'll send it to you, but it contains many of our own packages and we use also a Klondike server as backend. i'll send it to you via mail.

Contributor

Stift commented Mar 17, 2016

I'll send it to you, but it contains many of our own packages and we use also a Klondike server as backend. i'll send it to you via mail.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

i guess this means 2.52.14 is still broken...

Member

forki commented Mar 17, 2016

i guess this means 2.52.14 is still broken...

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

It has the same strange warning as before, but this time it does not tries to redownload.

Contributor

Stift commented Mar 17, 2016

It has the same strange warning as before, but this time it does not tries to redownload.

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

I have sent you our dep file per mail.

Contributor

Stift commented Mar 17, 2016

I have sent you our dep file per mail.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

just trying to reproduce with your deps file, but I assume in order to make it really hard the conflict is only in your custome nugets ;-)

Member

forki commented Mar 17, 2016

just trying to reproduce with your deps file, but I assume in order to make it really hard the conflict is only in your custome nugets ;-)

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 17, 2016

Contributor

no worries. I can pin paket for now. For today I'm out. I have some assumption on that as well. Before monday I will not get into that. Would be glad if you can give me a hint what I should look at. Lately I saw on the verbose level that one package was repeatedly tried to "retrieve information".

Contributor

Stift commented Mar 17, 2016

no worries. I can pin paket for now. For today I'm out. I have some assumption on that as well. Before monday I will not get into that. Would be glad if you can give me a hint what I should look at. Lately I saw on the verbose level that one package was repeatedly tried to "retrieve information".

@forki forki reopened this Mar 17, 2016

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 17, 2016

Member

so the public part of your deps file resolves without error. I guess we need to look into this next week.

Member

forki commented Mar 17, 2016

so the public part of your deps file resolves without error. I guess we need to look into this next week.

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 18, 2016

Contributor

@forki what do you think to separate releases into stable and beta channels because these frequent changes with high potential for regression, are affecting people day to day work.

Contributor

tsibelman commented Mar 18, 2016

@forki what do you think to separate releases into stable and beta channels because these frequent changes with high potential for regression, are affecting people day to day work.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 18, 2016

Member

we usally do that, but the v3 branch is blocking the alpha channel.
I aslo starting to add property based test around this part.

Member

forki commented Mar 18, 2016

we usally do that, but the v3 branch is blocking the alpha channel.
I aslo starting to add property based test around this part.

@tsibelman

This comment has been minimized.

Show comment
Hide comment
@tsibelman

tsibelman Mar 18, 2016

Contributor

Maybe you need 3 channels, stable, alpha for bigger longer changes and beta for smaller one

Contributor

tsibelman commented Mar 18, 2016

Maybe you need 3 channels, stable, alpha for bigger longer changes and beta for smaller one

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 18, 2016

Contributor

Ok. I think I found the error in our package that was causing the warning:

Could not resolve package Npgsql:
 - Dependencies file requested: 2.2.5-custom
 - <customPackage1> 1.0.11044 requested: >= 0
 - <customPackage2> 1.0.11149 requested: >= 2.2.5
 - <customPackage3> 1.1.10087 requested: >= 2.0.0 < 3.0.0

The <customPackage2> had its dependency too strict and therefore the Npgsql.2.2.5-custom was not fitting in its constraint. It was obvious after I saw it. The first time I misread the warning and thought that the customPackages were requested, but this morning I just discovered that it was meant in a way that this package requested npgsql of this version.

Contributor

Stift commented Mar 18, 2016

Ok. I think I found the error in our package that was causing the warning:

Could not resolve package Npgsql:
 - Dependencies file requested: 2.2.5-custom
 - <customPackage1> 1.0.11044 requested: >= 0
 - <customPackage2> 1.0.11149 requested: >= 2.2.5
 - <customPackage3> 1.1.10087 requested: >= 2.0.0 < 3.0.0

The <customPackage2> had its dependency too strict and therefore the Npgsql.2.2.5-custom was not fitting in its constraint. It was obvious after I saw it. The first time I misread the warning and thought that the customPackages were requested, but this morning I just discovered that it was meant in a way that this package requested npgsql of this version.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 18, 2016

Member

So what paket says is correct?

Member

forki commented Mar 18, 2016

So what paket says is correct?

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 18, 2016

Contributor

Yes. I misunderstood the message! Sorry for that confusion.

If I read the message like this, I would maybe understand this earlier.
Could not resolve package Npgsql:
- Dependencies file requested package Npgsql: 2.2.5-custom
- 1.0.11044 requested package Npgsql: >= 0
- 1.0.11149 requested package Npgsql: >= 2.2.5
- 1.1.10087 requested package Npgsql: >= 2.0.0 < 3.0.0

Should I make a pr for this?

Contributor

Stift commented Mar 18, 2016

Yes. I misunderstood the message! Sorry for that confusion.

If I read the message like this, I would maybe understand this earlier.
Could not resolve package Npgsql:
- Dependencies file requested package Npgsql: 2.2.5-custom
- 1.0.11044 requested package Npgsql: >= 0
- 1.0.11149 requested package Npgsql: >= 2.2.5
- 1.1.10087 requested package Npgsql: >= 2.0.0 < 3.0.0

Should I make a pr for this?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 18, 2016

Member

Yes everything that improves error messages is good.
On Mar 18, 2016 18:03, "Christian Fiebrig" notifications@github.com wrote:

Yes. I misunderstood the message! Sorry for that confusion.

If I read the message like this, I would maybe understand this earlier.
Could not resolve package Npgsql:

  • Dependencies file requested package Npgsql: 2.2.5-custom
  • 1.0.11044 requested package Npgsql: >= 0
  • 1.0.11149 requested package Npgsql: >= 2.2.5
  • 1.1.10087 requested package Npgsql: >= 2.0.0 < 3.0.0

Should I make a pr for this?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1524 (comment)

Member

forki commented Mar 18, 2016

Yes everything that improves error messages is good.
On Mar 18, 2016 18:03, "Christian Fiebrig" notifications@github.com wrote:

Yes. I misunderstood the message! Sorry for that confusion.

If I read the message like this, I would maybe understand this earlier.
Could not resolve package Npgsql:

  • Dependencies file requested package Npgsql: 2.2.5-custom
  • 1.0.11044 requested package Npgsql: >= 0
  • 1.0.11149 requested package Npgsql: >= 2.2.5
  • 1.1.10087 requested package Npgsql: >= 2.0.0 < 3.0.0

Should I make a pr for this?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1524 (comment)

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 22, 2016

Member

@Stift seems that I need your help again. Property based testing revealed another issue around that code. I was able to fix that but I want to know if how bad the perf is. Can you please check 3.0.0-alpha079? Thanks mate

Member

forki commented Mar 22, 2016

@Stift seems that I need your help again. Property based testing revealed another issue around that code. I was able to fix that but I want to know if how bad the perf is. Can you please check 3.0.0-alpha079? Thanks mate

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 22, 2016

Contributor

No problem.
stats for paket install

Commit v2.54.0 v3.0.0-alpha079
Unclean (with error in nupkg) 24s 2h+
Clean (paket.lock is fine) 8s 8s
Clean (paket.lock has to be updated) 11s 13s

and for a paket update nuget package only on a clean commit

v2.54.0 v3.0.0-alpha079
161s 161s

The unclean commit was the one that originally made problems and where we had a package which was not configured well.

I mean the only issue that I see is that with this unclean package we get into a situation where paket does not behave that well. When I looked in the verbose output of 2.52.10 (where the original problem started) I had the feeling paket was running in circles.

Contributor

Stift commented Mar 22, 2016

No problem.
stats for paket install

Commit v2.54.0 v3.0.0-alpha079
Unclean (with error in nupkg) 24s 2h+
Clean (paket.lock is fine) 8s 8s
Clean (paket.lock has to be updated) 11s 13s

and for a paket update nuget package only on a clean commit

v2.54.0 v3.0.0-alpha079
161s 161s

The unclean commit was the one that originally made problems and where we had a package which was not configured well.

I mean the only issue that I see is that with this unclean package we get into a situation where paket does not behave that well. When I looked in the verbose output of 2.52.10 (where the original problem started) I had the feeling paket was running in circles.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 22, 2016

Member

So apart from a strange state it was ok?

Member

forki commented Mar 22, 2016

So apart from a strange state it was ok?

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 22, 2016

Contributor

yes. Seems like current stable and alpha are similiar.

Contributor

Stift commented Mar 22, 2016

yes. Seems like current stable and alpha are similiar.

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 22, 2016

Contributor

I just fear a bad user experience when paket cannot detect that it runs in a regression.

Contributor

Stift commented Mar 22, 2016

I just fear a bad user experience when paket cannot detect that it runs in a regression.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 22, 2016

Member

Yes we need to find that error. Question is how to reproduce on my machine.
On Mar 22, 2016 8:56 PM, "Christian Fiebrig" notifications@github.com
wrote:

I just fear a bad user experience when paket cannot detect that it runs in
a regression.


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#1524 (comment)

Member

forki commented Mar 22, 2016

Yes we need to find that error. Question is how to reproduce on my machine.
On Mar 22, 2016 8:56 PM, "Christian Fiebrig" notifications@github.com
wrote:

I just fear a bad user experience when paket cannot detect that it runs in
a regression.


You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub
#1524 (comment)

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 23, 2016

Member

ok I added another 3.0alpha package. I hope it solves that 2h hour thing

Member

forki commented Mar 23, 2016

ok I added another 3.0alpha package. I hope it solves that 2h hour thing

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 23, 2016

Contributor

I tested with 3.0.0-alpha80 and with the unclean package paket install takes about 27 seconds. On a clean repo without the failing package we still have around 15 seconds. For me this looks definetely better and acceptable (from a user perspective).

Update is also fine on the clean one.

Contributor

Stift commented Mar 23, 2016

I tested with 3.0.0-alpha80 and with the unclean package paket install takes about 27 seconds. On a clean repo without the failing package we still have around 15 seconds. For me this looks definetely better and acceptable (from a user perspective).

Update is also fine on the clean one.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Mar 23, 2016

Member

thanks buddy.

Member

forki commented Mar 23, 2016

thanks buddy.

@Stift

This comment has been minimized.

Show comment
Hide comment
@Stift

Stift Mar 23, 2016

Contributor

no problem. Glad to serve and give something back.

Contributor

Stift commented Mar 23, 2016

no problem. Glad to serve and give something back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment