poetry install hugely slowed down by deepcopy operations #7459

Activity

1. ralbertazzi

   added

   kind/bugSomething isn't working as expectedSomething isn't working as expected

   status/triageThis issue needs to be triagedThis issue needs to be triaged

   on Feb 2, 2023
2. neersighted commented on Feb 2, 2023

   

   neersighted

   on Feb 2, 2023

   Member

   More actions

   The deepcopy operations are part of the solver and not an extraneous/non-core bit of Poetry. Work to improve performance is of course welcome (and you might have noticed from release notes that we have made significant strides there recently), but please note that "remove these expensive deepcopies" is probably not something that is possible with my understanding of the current architecture.

   Something like "evaluate how many copies are truly necessary vs. defensive in the solver" is much more tenable, but is also much more involved.

   👍React with 👍1Reacted by Riccardo Albertazzi
3. ralbertazzi commented on Feb 2, 2023

   

   ralbertazzi

   on Feb 2, 2023

   ContributorAuthor

   More actions

   I managed to create a reproducible project made of public dependencies, which you can find at this gist.

   Steps to reproduce:

   poetry install  # will install dependencies
   poetry install  # everything should be cached, instead it still takes 20 seconds
   

   The command is still deepcopy heavy, as you can see from the cProfile output
4. neersighted commented on Feb 2, 2023

   

   neersighted

   on Feb 2, 2023

   Member

   More actions

   Poetry is running the solver when it loads the LockfileRepository to verify the lock file is valid (e.g. doesn't have conflicting packages/will not result in an obvious nonsense solution). Invalid lock files are something that can happen e.g. from those who remove the hash and git merge so we can't not perform a solver run during install.

   See #496 for more context on this.
5. ralbertazzi commented on Feb 2, 2023

   

   ralbertazzi

   on Feb 2, 2023

   ContributorAuthor

   More actions

   @neersighted I thank you for your incredible contribution to the project and recent improvements (really looking forward 1.4 btw!). As I understand that touching those deepcopies mustn't be the easiest thing ever, I was still wondering if there was some low hanging fruit since the slowdown happens when everything has already been solved.

   As I was writing this comment, I received your reply above :) I'm not really a fan of supporting incorrect usages of the poetry.lock file by slowing down those who don't touch it, but I understand you somehow need to make everyone happy. Is there a way (or can we implement a simple command line option) to disable lockfile verification on poetry install? This simple option would speed up my CI pipelines by far! While I would personally disable the verification by default, I'd also be happy to just have the option to disable it when needed. Thanks a lot for your quick reply!
6. neersighted commented on Feb 2, 2023

   

   neersighted

   on Feb 2, 2023

   Member

   More actions

   I'll leave that to @radoering and @dimbleby; however, I don't think that's something we will accept upstream. The support burden of adding a knob that says "be less correct" when many users already subvert the protections we have in place (like the hash) would be massive, and it's not something any upstream developer would use or reason in terms of.
7. dimbleby commented on Feb 2, 2023

   

   dimbleby

   on Feb 2, 2023

   Contributor

   More actions

   re-solving is baked in to how poetry install works, it's not a thing that can just be disabled.

   In some sense it's best not to think of poetry.lock as a lockfile at all, it's more like a (greatly) cut-down copy of a repository.

   poetry install can't just read the answer out of poetry.lock: because that file simply doesn't contain that answer. Rather poetry install must freshly calculate a solution that's suitable for the current environment, using only the packages described by poetry.lock.

   Normally speaking this is fast because there are so few choices for each package in poetry.lock that there's approximately no branching at all.

   Of course merge requests bringing performance improvements are welcome... While I don't think there's enormous amounts of low-hanging fruit, I would bet that a sufficiently motivated person could make meaningful gains - and I could absolutely believe that all the deepcopy is a candidate for that sort of work.

   👍React with 👍1Reacted by Riccardo Albertazzi
8. Bobronium commented on Feb 2, 2023

   

   Bobronium

   on Feb 2, 2023

   Last edited by Bobronium

   More actions

   This might be relevant: I'm writing deepcopy replacement which is currently 20-50 times faster than builtin deepcopy on subsequent copies of the same object.

   If poetry workflow involves a scenario when the same object is copied multiple times, like so:

   for i in range(100):
       mutate(deepcopy(data))

   Replacing it with this would give you instant performance improvement:

   data_copier = duper.deepdups(data)
   for i in range(100):
       mutate(data_copier())

   Source:
   https://github.com/Bobronium/duper

   👍React with 👍1Reacted by themanifold