openSUSE / libsolv

Library for solving packages and reading repositories
http://en.opensuse.org/openSUSE:Libzypp_satsolver
Other
517 stars 152 forks source link

Deduplicate multiple subsequent `choicerule_find_installed` calls #545

Open jonashaag opened 10 months ago

jonashaag commented 10 months ago

Not sure if this is a correct change to make, I'm really new to the libsolv codebase.

It speeds up solving in Mamba by a factor of 5 for some environments.

cc @AntoinePrv do you have enough understanding of libsolv to tell if this is a correct change?

xref https://github.com/mamba-org/mamba/issues/2824#issuecomment-1722268274

Example:

mlschroe commented 10 months ago

Do you really need choicerules for mamba? They only make sense for packages providing multiple capabilities. I thought, conda doesn't do that. I could easily add a flag to turn off choice rule generation if they are not needed.

mlschroe commented 10 months ago

(Can you make a solver testcase available somewhere if mamba offers that? i.e. a call to testcase_write() after the solver run)

jonashaag commented 10 months ago

Do you really need choicerules for mamba?

There is no mention of choicerules in the codebase but I don't really know what it means so honestly I have no clue.

mlschroe commented 10 months ago

They are not visible to the outside, so there's no documentation except some comments in the code. Here's what they do: Many packaging systems allow a package to provide multiple capabilities. For example, the "postfix" and the "sendmail" package might both provide a "smtp_server" capability.

Libsolv tries to keep the installed packages unchanged (unless you tell it to update). To do so, it first resolves the rules coming from the user job, then it resolves all installed packages, and finally it resolves all open dependencies.

This is where choice rules come into play. Often a dependency may be solved by updating a already installed package. But without choicerules, libsolv will not update (because that would mean backtracking), but instead install a different provider. So it may install "sendmail" instead of updating "postfix".

The choice rules are basically normal dependencies rules with the new packages removed from the possibilities. They are marked as "weak", so they can be broken if needed. But they make the solver backtrack and try a package update to solve a dependency.

mlschroe commented 10 months ago

choicerule creation never took much time in Fedora/SUSE, so there must be something special in the conda repos to slow down solving that much.

jonashaag commented 10 months ago

Yeah I don't think that's a thing in Conda universe @AntoinePrv @wolfv?

Solving a medium sized Conda env performs 1M+ calls to choicerule_find_installed. Not sure why, that's just what I experimentally observed. For each package (s->name) we have on average 1.7k repeated (duplicated?) calls into the function.

wolfv commented 10 months ago

Nope, it's not a thing in the condaverse. At least not yet! :)

jonashaag commented 10 months ago

@mlschroe what next steps do you suggest? Happy with a flag that disables choice rules

mlschroe commented 10 months ago

There are two possibilities:

1) add a new SOLVER_FLAG_NO_CHOICERULES solver flag

2) never create choice rules if the disttype is DISTTYPE_CONDA

What do you prefer?

wolfv commented 10 months ago

Maybe better to disable that for DISTTYPE_CONDA instead of another compiler flag? :)

jonashaag commented 10 months ago

@AntoinePrv @wolfv any opinion?

AntoinePrv commented 10 months ago

I'm not so knowledgeable about it, but I would say a compiler flag is safer in case we turn out to need it.

jonashaag commented 10 months ago

@mlschroe I'm going to send a PR. Are you still interested in the "cache" I introduced in this PR?

mlschroe commented 10 months ago

No, the cache wont work in the general case when there are package obsoletes.

mlschroe commented 10 months ago

(Not saying that a cache wouldn't be a good thing, it just needs to be a bit more complicated. That's why I asked for a solver testcase, so that I can test things locally)

jonashaag commented 10 months ago

I wanted to provide a test case but I failed building/linking with Mamba with the extension. Cmake is hard.