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.

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

Deduplicate multiple subsequent choicerule_find_installed calls #545

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jonashaag
Copy link
Contributor

@jonashaag jonashaag commented Nov 26, 2023

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 mamba-org/mamba#2824 (comment)

Example:

  • Create environment from here
  • Run micromamba -p ... install xtensor
  • Before: 7.25 s
  • After: 3.20 s

@jonashaag jonashaag marked this pull request as draft November 26, 2023 11:58
@mlschroe
Copy link
Member

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
Copy link
Member

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

@jonashaag
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

jonashaag commented Nov 27, 2023

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
Copy link
Contributor

wolfv commented Nov 27, 2023

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

@jonashaag
Copy link
Contributor Author

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

@mlschroe
Copy link
Member

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
Copy link
Contributor

wolfv commented Nov 28, 2023

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

@jonashaag
Copy link
Contributor Author

@AntoinePrv @wolfv any opinion?

@AntoinePrv
Copy link
Contributor

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
Copy link
Contributor Author

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

@mlschroe
Copy link
Member

mlschroe commented Dec 5, 2023

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

@mlschroe
Copy link
Member

mlschroe commented Dec 5, 2023

(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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants